-
Notifications
You must be signed in to change notification settings - Fork 121
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
[TRA-471] update shares in vault query response to use NumShares type #1834
Conversation
WalkthroughThe primary update involves modifying the Changes
Sequence Diagram(s)The changes are mainly about type updates and do not alter the control flow significantly. As such, sequence diagrams are not applicable here. Poem
Tip AI model upgrade
|
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: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
protocol/x/vault/types/query.pb.go
is excluded by!**/*.pb.go
Files selected for processing (4)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/query.ts (7 hunks)
- proto/dydxprotocol/vault/query.proto (1 hunks)
- protocol/x/vault/keeper/grpc_query_vault.go (1 hunks)
- protocol/x/vault/keeper/grpc_query_vault_test.go (1 hunks)
Additional context used
Biome
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/query.ts
[error] 9-9: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
Additional comments not posted (5)
protocol/x/vault/keeper/grpc_query_vault.go (1)
57-57
: Update looks good. EnsureNumShares
type is correctly handled elsewhere.The assignment of
totalShares
directly toTotalShares
in theQueryVaultResponse
appears correct. Ensure that theNumShares
type is correctly handled in other parts of the code.proto/dydxprotocol/vault/query.proto (1)
63-63
: Update looks good. EnsureNumShares
type is correctly handled elsewhere.The change of the
total_shares
field to theNumShares
type in theQueryVaultResponse
message appears correct. Ensure that theNumShares
type is correctly handled in other parts of the code.protocol/x/vault/keeper/grpc_query_vault_test.go (1)
132-132
: Update looks good. EnsureNumShares
type is correctly handled elsewhere.The change to use
vaulttypes.BigIntToNumShares(tc.totalShares)
for theTotalShares
field in the expected response appears correct. Ensure that theNumShares
type is correctly handled in other parts of the code.indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/query.ts (2)
42-42
: Update looks good. EnsureNumShares
type is correctly handled elsewhere.The change of the
totalShares
field to theNumShares
type in theQueryVaultResponse
interface appears correct. Ensure that theNumShares
type is correctly handled in other parts of the code.
51-51
: Update looks good. EnsureNumShares
type is correctly handled elsewhere.The change of the
total_shares
field to theNumSharesSDKType
type in theQueryVaultResponseSDKType
interface appears correct. Ensure that theNumSharesSDKType
type is correctly handled in other parts of the code.
import { PageRequest, PageRequestSDKType, PageResponse, PageResponseSDKType } from "../../cosmos/base/query/v1beta1/pagination"; | ||
import { Params, ParamsSDKType } from "./params"; | ||
import { SubaccountId, SubaccountIdSDKType } from "../subaccounts/subaccount"; | ||
import * as _m0 from "protobufjs/minimal"; | ||
import { DeepPartial, Long } from "../../helpers"; | ||
import { DeepPartial } from "../../helpers"; | ||
/** QueryParamsRequest is a request type for the Params RPC method. */ | ||
|
||
export interface QueryParamsRequest {} |
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.
Avoid using empty interfaces.
An empty interface is equivalent to {}
. Use a type alias instead.
- export interface QueryParamsRequest {}
+ export type QueryParamsRequest = {};
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 QueryParamsRequest {} | |
export type QueryParamsRequest = {}; |
Tools
Biome
[error] 9-9: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
Changelist
Have
total_shares
in vault query response asNumShares
typeTest Plan
unit testing
localnet testing
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
totalShares
field in the Vault query responses to use a more precise custom type, improving data accuracy and consistency in vault share calculations.Bug Fixes
totalShares
field to prevent potential errors related to data type conversions.Tests
totalShares
field, ensuring robustness and preventing future regressions.