[CORE-826] Initial protos for x/ibcratelimit
#864
Conversation
WalkthroughThe updates involve the introduction of new protocol buffer definitions and gRPC services for the 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: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (5)
- protocol/x/ibcratelimit/types/capacity.pb.go
- protocol/x/ibcratelimit/types/genesis.pb.go
- protocol/x/ibcratelimit/types/limit_params.pb.go
- protocol/x/ibcratelimit/types/query.pb.go
- protocol/x/ibcratelimit/types/tx.pb.go
Files selected for processing (5)
- proto/dydxprotocol/ibcratelimit/capacity.proto (1 hunks)
- proto/dydxprotocol/ibcratelimit/genesis.proto (1 hunks)
- proto/dydxprotocol/ibcratelimit/limit_params.proto (1 hunks)
- proto/dydxprotocol/ibcratelimit/query.proto (1 hunks)
- proto/dydxprotocol/ibcratelimit/tx.proto (1 hunks)
Additional comments: 10
proto/dydxprotocol/ibcratelimit/capacity.proto (1)
- 9-20: The
DenomCapacitymessage is well-defined with clear comments explaining the purpose of each field. The use ofbytesforcapacity_listwith a custom typeSerializableIntis noted, which should ensure that the capacity amounts are serialized efficiently. Thenullableoption is set tofalse, which is appropriate for repeated fields to avoid unnecessary null checks.proto/dydxprotocol/ibcratelimit/genesis.proto (1)
- 9-13: The
GenesisStatemessage definition correctly includes a repeated fieldlimit_params_listof typeLimitParamswith thegogoproto.nullableoption set to false, ensuring that the list cannot contain null elements.proto/dydxprotocol/ibcratelimit/limit_params.proto (1)
- 9-35: The
LimitParamsandLimitermessage definitions are well-structured and align with the PR's objective to define rate limiting parameters. The use ofbytesforbaseline_minimumwith a custom typeSerializableIntensures that large integer values can be handled correctly. Ensure that the custom typeSerializableIntis well-documented and tested, as it is critical for the correct serialization and deserialization of these values.proto/dydxprotocol/ibcratelimit/query.proto (4)
10-18: The gRPC service
Queryis correctly defined with the two RPC methodsListLimitParamsandCapacityByDenom. The methods are well-documented, indicating their purpose, which aligns with the PR's objective to introduce a global withdrawal speed limit feature.20-26: The
ListLimitParamsRequestandListLimitParamsResponsemessages are defined correctly. Thelimit_params_listfield inListLimitParamsResponseis a repeated field ofLimitParams, which is consistent with the summary provided.28-46: The
QueryCapacityByDenomRequestandQueryCapacityByDenomResponsemessages are defined correctly. TheQueryCapacityByDenomRequestincludes adenomfield for the token denomination, and theQueryCapacityByDenomResponseincludes a repeated fieldresultsof typeCapacityResult, which aligns with the summary provided.33-40: The
CapacityResultmessage uses a custom type for thecapacityfield, which is specified to be non-nullable. Ensure that the custom typegithub.com/dydxprotocol/v4-chain/protocol/dtypes.SerializableIntis correctly implemented and that all parts of the codebase that interact withCapacityResultare aware of how to handle this custom type.
The usage of the custom type
SerializableIntin theCapacityResultmessage is consistent with its implementation and usage throughout the codebase. No issues found.proto/dydxprotocol/ibcratelimit/tx.proto (3)
10-16: The
Msgservice definition correctly introduces theSetLimitParamsandDeleteLimitParamsRPC methods as described in the summary.18-26: The
MsgSetLimitParamsmessage is well-defined with anauthorityfield to specify the signer and alimit_paramsfield to define the parameters to set.31-39: The
MsgDeleteLimitParamsmessage is correctly structured with anauthorityfield for the signer and adenomfield to identify theLimitParamsto be deleted.
| // SetLimitParams sets a `LimitParams` object in state. | ||
| rpc SetLimitParams(MsgSetLimitParams) returns (MsgSetLimitParamsResponse); | ||
|
|
||
| // DeleteLimitParams removes a `LimitParams` object in state. |
There was a problem hiding this comment.
mega nit:
| // DeleteLimitParams removes a `LimitParams` object in state. | |
| // DeleteLimitParams removes a `LimitParams` object from state. |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 7
Configuration used: CodeRabbit UI
Files ignored due to filter (3)
- protocol/x/ibcratelimit/types/capacity.pb.go
- protocol/x/ibcratelimit/types/query.pb.go
- protocol/x/ibcratelimit/types/tx.pb.go
Files selected for processing (16)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/bundle.ts (3 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/ibcratelimit/capacity.ts (1 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/ibcratelimit/genesis.ts (1 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/ibcratelimit/limit_params.ts (1 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/ibcratelimit/query.rpc.Query.ts (1 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/ibcratelimit/query.ts (1 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/ibcratelimit/tx.rpc.msg.ts (1 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/ibcratelimit/tx.ts (1 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/rpc.query.ts (1 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/rpc.tx.ts (1 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/ibcratelimit/capacity.proto (1 hunks)
- proto/dydxprotocol/ibcratelimit/limit_params.proto (1 hunks)
- proto/dydxprotocol/ibcratelimit/query.proto (1 hunks)
- proto/dydxprotocol/ibcratelimit/tx.proto (1 hunks)
Files skipped from review due to trivial changes (2)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/bundle.ts
- indexer/packages/v4-protos/src/codegen/gogoproto/bundle.ts
Files skipped from review as they are similar to previous changes (4)
- proto/dydxprotocol/ibcratelimit/capacity.proto
- proto/dydxprotocol/ibcratelimit/limit_params.proto
- proto/dydxprotocol/ibcratelimit/query.proto
- proto/dydxprotocol/ibcratelimit/tx.proto
Additional comments: 13
indexer/packages/v4-protos/src/codegen/dydxprotocol/ibcratelimit/capacity.ts (4)
5-18: The TypeScript interface
DenomCapacityis correctly defined with appropriate comments explaining the properties. ThecapacityListis an array ofUint8Array, which aligns with the expected protobuf byte array type for representing lists of binary data.21-34: The
DenomCapacitySDKTypeinterface is consistent with theDenomCapacityinterface, with the propertycapacity_listfollowing the snake_case naming convention typically used in protobuf definitions. This ensures consistency between the TypeScript code and the protobuf definitions.36-41: The
createBaseDenomCapacityfunction provides a clean and clear way to create a baseDenomCapacityobject with default values. This is a good practice for initializing objects that will be populated later, either through decoding or partial object updates.43-87: The
DenomCapacityclass containsencode,decode, andfromPartialmethods which are essential for working with protobuf data in TypeScript. These methods are implemented correctly, handling the encoding of non-empty strings and arrays of bytes, as well as decoding and constructing partial objects from provided values.indexer/packages/v4-protos/src/codegen/dydxprotocol/ibcratelimit/genesis.ts (1)
- 1-59: The changes to the
GenesisStateandGenesisStateSDKTypeinterfaces, along with their associated methods, are consistent with the PR objectives and the summary provided. The code is well-structured and follows the conventions for defining protobuf-related types and methods in TypeScript. The encode, decode, and fromPartial methods are correctly implemented to handle thelimitParamsListfield.indexer/packages/v4-protos/src/codegen/dydxprotocol/ibcratelimit/query.rpc.Query.ts (5)
7-12: The interface
Querycorrectly defines the two methodslistLimitParamsandcapacityByDenomas described in the PR summary. The optional parameter inlistLimitParamsis consistent with the gRPC convention for request objects that may not require parameters.14-21: The
QueryClientImplclass properly implements theQueryinterface and binds the methods within the constructor to ensure the correctthiscontext when the methods are called. This is a good practice in TypeScript to avoid issues withthiscontext in callbacks.23-26: The
listLimitParamsmethod implementation correctly encodes the request, sends the gRPC request, and decodes the response. This is in line with the expected behavior for a gRPC client method.29-32: The
capacityByDenommethod follows the same pattern aslistLimitParamsfor encoding the request, sending the gRPC request, and decoding the response. This consistency in method implementation is good for maintainability.35-48: The
createRpcQueryExtensionfunction creates a newQueryClientImplinstance and returns an object with methods that proxy the calls to the service's methods. This pattern allows for extending theQueryClientwith additional methods, which is a flexible design choice.indexer/packages/v4-protos/src/codegen/dydxprotocol/rpc.query.ts (1)
- 19-19: The addition of the
ibcratelimitmodule to thecreateRPCQueryClientfunction is consistent with the PR objectives and the provided summaries. This change is necessary to support the newx/ibcratelimitmodule in the dydxprotocol.indexer/packages/v4-protos/src/codegen/dydxprotocol/rpc.tx.ts (1)
- 13-13: The addition of the
ibcratelimitmodule to thecreateRPCMsgClientfunction is consistent with the PR objectives and the AI-generated summaries. This change integrates the new rate limiting functionality into the RPC message client creation process.indexer/packages/v4-protos/src/codegen/google/bundle.ts (1)
- 1-16: The changes to the import statements and namespace declarations are consistent with the summary provided. The import numbers have been incremented, which aligns with the addition of new modules or reorganization of the codebase. Ensure that these changes do not break any references in the codebase where these namespaces are used.
| export interface LimiterSDKType { | ||
| /** | ||
| * period_sec is the rolling time period for which the limit applies | ||
| * e.g. 3600 (an hour) | ||
| */ | ||
| period_sec: number; |
There was a problem hiding this comment.
The naming convention for period_sec in LimiterSDKType should be consistent with the Limiter interface. Consider renaming it to periodSec to match the camelCase convention used in TypeScript.
export interface LimiterSDKType {
/**
* period_sec is the rolling time period for which the limit applies
* e.g. 3600 (an hour)
*/
- period_sec: number;
+ periodSec: number;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.
| export interface LimiterSDKType { | |
| /** | |
| * period_sec is the rolling time period for which the limit applies | |
| * e.g. 3600 (an hour) | |
| */ | |
| period_sec: number; | |
| export interface LimiterSDKType { | |
| /** | |
| * periodSec is the rolling time period for which the limit applies | |
| * e.g. 3600 (an hour) | |
| */ | |
| periodSec: number; |
| * e.g. 100_000_000_000 uusdc for 100k USDC; 5e22 adv4tnt for 50k DV4TNT | ||
| */ | ||
|
|
||
| baselineMinimum: Uint8Array; |
There was a problem hiding this comment.
Consider adding a comment to clarify the expected format of the baselineMinimum byte array in the Limiter interface.
+ // baselineMinimum should be encoded as a byte array representing the numeric value.
baselineMinimum: Uint8Array;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.
| baselineMinimum: Uint8Array; | |
| // baselineMinimum should be encoded as a byte array representing the numeric value. | |
| baselineMinimum: Uint8Array; |
| /** ListLimitParamsRequest is a request type of the ListLimitParams RPC method. */ | ||
|
|
||
| export interface ListLimitParamsRequest {} | ||
| /** ListLimitParamsRequest is a request type of the ListLimitParams RPC method. */ |
There was a problem hiding this comment.
There are duplicate comments above the ListLimitParamsRequest interface. Consider removing the duplicate to improve code clarity.
- /** ListLimitParamsRequest is a request type of the ListLimitParams RPC method. */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.
| /** ListLimitParamsRequest is a request type of the ListLimitParams RPC method. */ |
| setLimitParams(request: MsgSetLimitParams): Promise<MsgSetLimitParamsResponse> { | ||
| const data = MsgSetLimitParams.encode(request).finish(); | ||
| const promise = this.rpc.request("dydxprotocol.ibcratelimit.Msg", "SetLimitParams", data); | ||
| return promise.then(data => MsgSetLimitParamsResponse.decode(new _m0.Reader(data))); | ||
| } | ||
|
|
||
| deleteLimitParams(request: MsgDeleteLimitParams): Promise<MsgDeleteLimitParamsResponse> { | ||
| const data = MsgDeleteLimitParams.encode(request).finish(); | ||
| const promise = this.rpc.request("dydxprotocol.ibcratelimit.Msg", "DeleteLimitParams", data); | ||
| return promise.then(data => MsgDeleteLimitParamsResponse.decode(new _m0.Reader(data))); | ||
| } |
There was a problem hiding this comment.
The MsgClientImpl class methods setLimitParams and deleteLimitParams do not include error handling. Consider adding try-catch blocks to handle potential rejections from the rpc.request method and provide a more robust implementation.
async setLimitParams(request: MsgSetLimitParams): Promise<MsgSetLimitParamsResponse> {
try {
const data = MsgSetLimitParams.encode(request).finish();
const response = await this.rpc.request("dydxprotocol.ibcratelimit.Msg", "SetLimitParams", data);
return MsgSetLimitParamsResponse.decode(new _m0.Reader(response));
} catch (error) {
// Handle or rethrow the error as appropriate
throw error;
}
}
async deleteLimitParams(request: MsgDeleteLimitParams): Promise<MsgDeleteLimitParamsResponse> {
try {
const data = MsgDeleteLimitParams.encode(request).finish();
const response = await this.rpc.request("dydxprotocol.ibcratelimit.Msg", "DeleteLimitParams", data);
return MsgDeleteLimitParamsResponse.decode(new _m0.Reader(response));
} catch (error) {
// Handle or rethrow the error as appropriate
throw error;
}
}| periodSec: number; | ||
| capacity: Uint8Array; | ||
| } | ||
| /** CapacityResult is a specific rate limit for a denom. */ |
There was a problem hiding this comment.
There are duplicate comments above the CapacityResult interface. Consider removing the duplicate to improve code clarity.
- /** CapacityResult is a specific rate limit for a denom. */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.
| /** CapacityResult is a specific rate limit for a denom. */ |
| /** MsgSetLimitParamsResponse is the Msg/SetLimitParams response type. */ | ||
|
|
||
| export interface MsgSetLimitParamsResponseSDKType {} | ||
| /** MsgDeleteLimitParams is the Msg/SetLimitParams request type. */ |
There was a problem hiding this comment.
The comment above MsgDeleteLimitParams incorrectly states that it is the Msg/SetLimitParams request type. This should be corrected to Msg/DeleteLimitParams.
- /** MsgDeleteLimitParams is the Msg/SetLimitParams request type. */
+ /** MsgDeleteLimitParams is the Msg/DeleteLimitParams request type. */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.
| /** MsgDeleteLimitParams is the Msg/SetLimitParams request type. */ | |
| /** MsgDeleteLimitParams is the Msg/DeleteLimitParams request type. */ |
|
|
||
| denom: string; | ||
| } | ||
| /** MsgDeleteLimitParams is the Msg/SetLimitParams request type. */ |
There was a problem hiding this comment.
Similarly, the comment above MsgDeleteLimitParamsSDKType also incorrectly states that it is the Msg/SetLimitParams request type. This should be corrected to Msg/DeleteLimitParams.
- /** MsgDeleteLimitParams is the Msg/SetLimitParams request type. */
+ /** MsgDeleteLimitParams is the Msg/DeleteLimitParams request type. */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.
| /** MsgDeleteLimitParams is the Msg/SetLimitParams request type. */ | |
| /** MsgDeleteLimitParams is the Msg/DeleteLimitParams request type. */ |
Changelist
Initial protos for
x/ibcratelimit. Design doc here. While it's not 100% finalized, wanted to check in the current version to get the ball rolling.Test Plan
N/A
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.