-
Notifications
You must be signed in to change notification settings - Fork 126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CT-834] new endpoint for stateful orders #1501
Conversation
WalkthroughThe update enhances the dYdX protocol with a new RPC Changes
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 Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
protocol/x/clob/types/query.pb.go
is excluded by!**/*.pb.go
,!**/*.pb.go
Files selected for processing (6)
- proto/dydxprotocol/clob/query.proto (2 hunks)
- protocol/mocks/QueryClient.go (1 hunks)
- protocol/x/clob/client/cli/query.go (1 hunks)
- protocol/x/clob/client/cli/query_stateful_order.go (1 hunks)
- protocol/x/clob/keeper/grpc_query_stateful_order.go (1 hunks)
- protocol/x/clob/keeper/grpc_query_stateful_order_test.go (1 hunks)
Files not reviewed due to errors (4)
- protocol/x/clob/client/cli/query.go (no review received)
- protocol/x/clob/keeper/grpc_query_stateful_order.go (no review received)
- protocol/x/clob/keeper/grpc_query_stateful_order_test.go (no review received)
- protocol/x/clob/client/cli/query_stateful_order.go (no review received)
Additional Context Used
Path-based Instructions (6)
protocol/x/clob/client/cli/query.go (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/x/clob/keeper/grpc_query_stateful_order.go (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/x/clob/keeper/grpc_query_stateful_order_test.go (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/x/clob/client/cli/query_stateful_order.go (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.proto/dydxprotocol/clob/query.proto (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/mocks/QueryClient.go (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.
// StatefulOrder provides a mock function with given fields: ctx, in, opts | ||
func (_m *QueryClient) StatefulOrder(ctx context.Context, in *clobtypes.QueryStatefulOrderRequest, opts ...grpc.CallOption) (*clobtypes.QueryStatefulOrderResponse, error) { | ||
_va := make([]interface{}, len(opts)) | ||
for _i := range opts { | ||
_va[_i] = opts[_i] | ||
} | ||
var _ca []interface{} | ||
_ca = append(_ca, ctx, in) | ||
_ca = append(_ca, _va...) | ||
ret := _m.Called(_ca...) | ||
|
||
if len(ret) == 0 { | ||
panic("no return value specified for StatefulOrder") | ||
} | ||
|
||
var r0 *clobtypes.QueryStatefulOrderResponse | ||
var r1 error | ||
if rf, ok := ret.Get(0).(func(context.Context, *clobtypes.QueryStatefulOrderRequest, ...grpc.CallOption) (*clobtypes.QueryStatefulOrderResponse, error)); ok { | ||
return rf(ctx, in, opts...) | ||
} | ||
if rf, ok := ret.Get(0).(func(context.Context, *clobtypes.QueryStatefulOrderRequest, ...grpc.CallOption) *clobtypes.QueryStatefulOrderResponse); ok { | ||
r0 = rf(ctx, in, opts...) | ||
} else { | ||
if ret.Get(0) != nil { | ||
r0 = ret.Get(0).(*clobtypes.QueryStatefulOrderResponse) | ||
} | ||
} | ||
|
||
if rf, ok := ret.Get(1).(func(context.Context, *clobtypes.QueryStatefulOrderRequest, ...grpc.CallOption) error); ok { | ||
r1 = rf(ctx, in, opts...) | ||
} else { | ||
r1 = ret.Error(1) | ||
} | ||
|
||
return r0, r1 | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure proper error handling for the StatefulOrder
mock function.
The StatefulOrder
function should handle the case where ret.Get(0)
or ret.Get(1)
is nil
to avoid potential runtime panics. This can be done by adding nil checks before type assertions.
if ret.Get(0) != nil {
r0 = ret.Get(0).(*clobtypes.QueryStatefulOrderResponse)
}
if ret.Get(1) != nil {
r1 = ret.Get(1).(error)
}
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.
// StatefulOrder provides a mock function with given fields: ctx, in, opts | |
func (_m *QueryClient) StatefulOrder(ctx context.Context, in *clobtypes.QueryStatefulOrderRequest, opts ...grpc.CallOption) (*clobtypes.QueryStatefulOrderResponse, error) { | |
_va := make([]interface{}, len(opts)) | |
for _i := range opts { | |
_va[_i] = opts[_i] | |
} | |
var _ca []interface{} | |
_ca = append(_ca, ctx, in) | |
_ca = append(_ca, _va...) | |
ret := _m.Called(_ca...) | |
if len(ret) == 0 { | |
panic("no return value specified for StatefulOrder") | |
} | |
var r0 *clobtypes.QueryStatefulOrderResponse | |
var r1 error | |
if rf, ok := ret.Get(0).(func(context.Context, *clobtypes.QueryStatefulOrderRequest, ...grpc.CallOption) (*clobtypes.QueryStatefulOrderResponse, error)); ok { | |
return rf(ctx, in, opts...) | |
} | |
if rf, ok := ret.Get(0).(func(context.Context, *clobtypes.QueryStatefulOrderRequest, ...grpc.CallOption) *clobtypes.QueryStatefulOrderResponse); ok { | |
r0 = rf(ctx, in, opts...) | |
} else { | |
if ret.Get(0) != nil { | |
r0 = ret.Get(0).(*clobtypes.QueryStatefulOrderResponse) | |
} | |
} | |
if rf, ok := ret.Get(1).(func(context.Context, *clobtypes.QueryStatefulOrderRequest, ...grpc.CallOption) error); ok { | |
r1 = rf(ctx, in, opts...) | |
} else { | |
r1 = ret.Error(1) | |
} | |
return r0, r1 | |
} | |
// StatefulOrder provides a mock function with given fields: ctx, in, opts | |
func (_m *QueryClient) StatefulOrder(ctx context.Context, in *clobtypes.QueryStatefulOrderRequest, opts ...grpc.CallOption) (*clobtypes.QueryStatefulOrderResponse, error) { | |
_va := make([]interface{}, len(opts)) | |
for _i := range opts { | |
_va[_i] = opts[_i] | |
} | |
var _ca []interface{} | |
_ca = append(_ca, ctx, in) | |
_ca = append(_ca, _va...) | |
ret := _m.Called(_ca...) | |
if len(ret) == 0 { | |
panic("no return value specified for StatefulOrder") | |
} | |
var r0 *clobtypes.QueryStatefulOrderResponse | |
var r1 error | |
if rf, ok := ret.Get(0).(func(context.Context, *clobtypes.QueryStatefulOrderRequest, ...grpc.CallOption) (*clobtypes.QueryStatefulOrderResponse, error)); ok { | |
return rf(ctx, in, opts...) | |
} | |
if rf, ok := ret.Get(0).(func(context.Context, *clobtypes.QueryStatefulOrderRequest, ...grpc.CallOption) *clobtypes.QueryStatefulOrderResponse); ok { | |
r0 = rf(ctx, in, opts...) | |
} else { | |
if ret.Get(0) != nil { | |
r0 = ret.Get(0).(*clobtypes.QueryStatefulOrderResponse) | |
} | |
} | |
if rf, ok := ret.Get(1).(func(context.Context, *clobtypes.QueryStatefulOrderRequest, ...grpc.CallOption) error); ok { | |
r1 = rf(ctx, in, opts...) | |
} else { | |
if ret.Get(1) != nil { | |
r1 = ret.Get(1).(error) | |
} | |
} | |
return r0, r1 | |
} |
proto/dydxprotocol/clob/query.proto
Outdated
// Queries the stateful order for a given order id. | ||
rpc StatefulOrder(QueryStatefulOrderRequest) returns (QueryStatefulOrderResponse) {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add HTTP annotations for the new RPC method.
Consider adding HTTP annotations to the StatefulOrder
RPC method for consistency with other methods and to enable REST API support.
rpc StatefulOrder(QueryStatefulOrderRequest) returns (QueryStatefulOrderResponse) {
option (google.api.http).get = "/dydxprotocol/clob/stateful_order/{order_id}";
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- proto/dydxprotocol/clob/query.proto (2 hunks)
- protocol/x/clob/module_test.go (1 hunks)
Files not reviewed due to errors (1)
- protocol/x/clob/module_test.go (no review received)
Files skipped from review as they are similar to previous changes (1)
- proto/dydxprotocol/clob/query.proto
Additional Context Used
Path-based Instructions (1)
protocol/x/clob/module_test.go (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.
// Fill amounts. | ||
uint64 fill_amount = 2; | ||
|
||
// Triggered status. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assuming this is false for regular stateful orders
return err | ||
} | ||
|
||
orderFlag, err := cast.ToUint32E(args[4]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we do some check on order flag it can only be like 32 or 64 i think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to over-engineer this. non 32/64 values will just return not found
"google.golang.org/grpc/status" | ||
) | ||
|
||
func (k Keeper) StatefulOrder( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
func (k Keeper) StatefulOrder( | |
func (k Keeper) QueryStatefulOrder( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has to match the GRPC query name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- 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)
Additional Context Used
Path-based Instructions (2)
indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/query.rpc.Query.ts (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/query.ts (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.
Additional comments not posted (4)
indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/query.rpc.Query.ts (1)
88-92
: The method implementation looks correct and follows the existing pattern for other query methods.indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/query.ts (3)
176-181
: The new request type definition forQueryStatefulOrderRequest
looks correct and follows the existing pattern for other request types.
188-216
: The new response type definition forQueryStatefulOrderResponse
looks correct and follows the existing pattern for other response types.
914-1022
: The encoding and decoding functions forQueryStatefulOrderRequest
andQueryStatefulOrderResponse
are implemented correctly and follow the existing pattern for other types.
/** Queries the stateful order for a given order id. */ | ||
|
||
statefulOrder(request: QueryStatefulOrderRequest): Promise<QueryStatefulOrderResponse>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a period at the end of the comment for consistency.
- /** Queries the stateful order for a given order id. */
+ /** Queries the stateful order for a given order id. */
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.
/** Queries the stateful order for a given order id. */ | |
statefulOrder(request: QueryStatefulOrderRequest): Promise<QueryStatefulOrderResponse>; | |
/** Queries the stateful order for a given order id. */ | |
statefulOrder(request: QueryStatefulOrderRequest): Promise<QueryStatefulOrderResponse>; |
@@ -40,6 +43,7 @@ | |||
this.equityTierLimitConfiguration = this.equityTierLimitConfiguration.bind(this); | |||
this.blockRateLimitConfiguration = this.blockRateLimitConfiguration.bind(this); | |||
this.liquidationsConfiguration = this.liquidationsConfiguration.bind(this); | |||
this.statefulOrder = this.statefulOrder.bind(this); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure consistent spacing around the method binding.
- this.statefulOrder = this.statefulOrder.bind(this);
+ this.statefulOrder = this.statefulOrder.bind(this);
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.
this.statefulOrder = this.statefulOrder.bind(this); |
@Mergifyio backport release/protocol/v4.1.x |
✅ Backports have been created
|
@Mergifyio backport release/protocol/v5.x |
✅ Backports have been created
|
* [CT-834] new endpoint for stateful orders * fix test * indexer (cherry picked from commit c24be3a) # Conflicts: # indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/query.rpc.Query.ts # indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/query.ts # protocol/x/clob/types/query.pb.go
* [CT-834] new endpoint for stateful orders * fix test * indexer (cherry picked from commit c24be3a) # Conflicts: # indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/query.rpc.Query.ts # indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/query.ts # protocol/x/clob/types/query.pb.go
* [CT-834] new endpoint for stateful orders * fix test * indexer (cherry picked from commit c24be3a)
* [CT-834] new endpoint for stateful orders * fix test * indexer (cherry picked from commit c24be3a)
* [CT-834] new endpoint for stateful orders * fix test * indexer (cherry picked from commit c24be3a) Co-authored-by: jayy04 <[email protected]>
* [CT-834] new endpoint for stateful orders * fix test * indexer (cherry picked from commit c24be3a) Co-authored-by: jayy04 <[email protected]>
* [CT-834] new endpoint for stateful orders * fix test * indexer (cherry picked from commit c24be3a) Co-authored-by: jayy04 <[email protected]>
* [CT-834] new endpoint for stateful orders * fix test * indexer (cherry picked from commit c24be3a) Co-authored-by: jayy04 <[email protected]>
* [CT-834] new endpoint for stateful orders * fix test * indexer (cherry picked from commit c24be3a) Co-authored-by: jayy04 <[email protected]>
* [CT-834] new endpoint for stateful orders * fix test * indexer (cherry picked from commit c24be3a) Co-authored-by: jayy04 <[email protected]>
* [CT-834] new endpoint for stateful orders * fix test * indexer (cherry picked from commit c24be3a) Co-authored-by: jayy04 <[email protected]>
Changelist
[Describe or list the changes made in this PR]
Test Plan
[Describe how this PR was tested (if applicable)]
Author/Reviewer Checklist
state-breaking
label.indexer-postgres-breaking
label.PrepareProposal
orProcessProposal
, manually add the labelproposal-breaking
.feature:[feature-name]
.backport/[branch-name]
.refactor
,chore
,bug
.Summary by CodeRabbit
New Features
StatefulOrder
to query stateful orders by order ID, enhancing order information retrieval.CmdQueryStatefulOrder()
to query stateful orders with specific parameters.Tests