-
Notifications
You must be signed in to change notification settings - Fork 124
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
[CLOB-1038] liquidation daemon - grpc to get all market prices #857
Conversation
WalkthroughThe codebase has been updated to enhance the liquidation daemon's capabilities. A new Changes
TipsChat with CodeRabbit Bot (
|
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (4)
- protocol/daemons/liquidation/client/client.go (3 hunks)
- protocol/daemons/liquidation/client/grpc_helper.go (5 hunks)
- protocol/daemons/liquidation/client/grpc_helper_test.go (2 hunks)
- protocol/lib/metrics/metric_keys.go (1 hunks)
Files skipped from review due to trivial changes (1)
- protocol/lib/metrics/metric_keys.go
Additional comments: 11
protocol/daemons/liquidation/client/client.go (2)
24-29: The addition of
PricesQueryClient
to theClient
struct aligns with the PR's objective to introduce gRPC client functionality for retrieving market prices.91-96: Initialization of
PricesQueryClient
within theStart
method is consistent with the PR's objective and the provided summaries.protocol/daemons/liquidation/client/grpc_helper.go (6)
93-100: The change from
limit
topageLimit
in theGetAllSubaccounts
function aligns with the PR objectives and summaries provided, indicating a shift towards a paginated approach for data retrieval.107-112: The
getSubaccountsFromKey
function has been correctly updated to acceptpageLimit
instead oflimit
, consistent with the changes inGetAllSubaccounts
.44-90: The implementation of the
GetAllMarketPrices
function correctly queries the gRPC server for market prices and handles pagination as described in the PR objectives and summaries.229-259: The
getMarketPricesFromKey
helper function has been added and is implemented to support the retrieval of market prices from the gRPC server using pagination, which is in line with the PR objectives and summaries.2-19: The hunks do not show the addition of the new constants
DaemonGetAllMarketPricesLatency
andDaemonGetMarketPricesPaginatedLatency
for latency metrics. If these constants were added in a different part of the file or another file, please ensure their usage is consistent with the telemetry and metrics patterns established in the codebase.2-19: The PR objectives mention a test plan with a new test function
TestGetAllMarketPrices
, but the provided hunks do not include any test code. Ensure that the test plan is implemented and that the new functionality is covered by tests, as per the PR objectives.protocol/daemons/liquidation/client/grpc_helper_test.go (3)
15-21: The addition of the
pricestypes
import aligns with the PR's objective to introduce gRPC client functionality for market price retrieval.179-271: The new test function
TestGetAllMarketPrices
is well-structured and covers multiple scenarios including success, pagination, and error propagation, which aligns with the PR's objective to ensure robust testing of the new market prices retrieval functionality.176-275: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [15-271]
No further issues or improvements are identified in the provided hunks.
Changelist
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
.