[TRA-380] use serializable int for equity and inventory in vault query #1688
Conversation
WalkthroughThe recent changes involve updating the types of Changes
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/query.ts (1)
Line range hint
9-9: The use of empty interfaces has been flagged by the static analysis tool. Consider using a type alias for better clarity and maintainability.type QueryParamsRequest = {}; type QueryParamsRequestSDKType = {};Also applies to: 12-12
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
protocol/x/vault/types/query.pb.gois excluded by!**/*.pb.go
Files selected for processing (4)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/query.ts (5 hunks)
- proto/dydxprotocol/vault/query.proto (1 hunks)
- protocol/x/vault/keeper/grpc_query_vault.go (2 hunks)
- protocol/x/vault/keeper/grpc_query_vault_test.go (3 hunks)
Additional context used
golangci-lint
protocol/x/vault/keeper/grpc_query_vault.go
18-18: undefined: Keeper (typecheck)
61-61: undefined: Keeper (typecheck)
Biome
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/query.ts
[error] 9-9: An empty interface is equivalent to {}. (lint/suspicious/noEmptyInterface)
Safe fix: Use a type alias instead.
[error] 12-12: An empty interface is equivalent to {}. (lint/suspicious/noEmptyInterface)
Safe fix: Use a type alias instead.
Additional comments not posted (7)
protocol/x/vault/keeper/grpc_query_vault.go (3)
12-12: The import ofdtypesis necessary for handling the new data types forequityandinventory. Ensure that this package is correctly versioned and maintained to avoid future compatibility issues.
55-56: The conversion ofequityandinventoryto usedtypes.NewIntFromBigIntaligns with the new data type requirements. This change ensures that negative values are correctly handled, which is crucial for financial calculations.
Line range hint
18-18: The static analysis tool has flagged an undefinedKeepertype. This might be a false positive ifKeeperis defined elsewhere in the project that wasn't included in the review materials. Please verify the definition ofKeeper.Also applies to: 61-61
proto/dydxprotocol/vault/query.proto (1)
54-63: The update to usebytestype with a custom serialization typeSerializableIntforequityandinventoryfields is a significant change. This ensures that these fields can handle large numbers and negative values more robustly than the previousuint64type.protocol/x/vault/keeper/grpc_query_vault_test.go (2)
Line range hint
34-59: The test cases have been updated to handle the new data types forequityandinventory. It's good to see both positive and negative scenarios being tested, which is crucial for financial applications to ensure robustness.
128-129: The use ofdtypes.NewIntFromBigIntin the test assertions is consistent with the changes in the main implementation. This consistency is key to ensuring that the tests accurately reflect the behavior of the production code.indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/query.ts (1)
40-41: The update to useUint8Arrayforequityandinventoryin TypeScript interfaces aligns with the changes in other parts of the system. This ensures type consistency across the stack, which is crucial for data integrity and error prevention.
[APROVED]Also applies to: 49-50
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (2)
protocol/x/vault/keeper/grpc_query_vault.go (1)
Line range hint
18-18: The static analysis tool has flagged an issue with theKeepertype being undefined. Please ensure that theKeepertype is correctly defined and accessible within this context.Also applies to: 61-61
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/query.ts (1)
Line range hint
9-9: The interfacesQueryParamsRequestandQueryParamsRequestSDKTypeare defined as empty, which is equivalent to{}. Consider using a type alias for clarity and to address the linting issue.type QueryParamsRequest = {}; type QueryParamsRequestSDKType = {};Also applies to: 12-12
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
protocol/x/vault/types/query.pb.gois excluded by!**/*.pb.go
Files selected for processing (4)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/query.ts (5 hunks)
- proto/dydxprotocol/vault/query.proto (1 hunks)
- protocol/x/vault/keeper/grpc_query_vault.go (2 hunks)
- protocol/x/vault/keeper/grpc_query_vault_test.go (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- proto/dydxprotocol/vault/query.proto
- protocol/x/vault/keeper/grpc_query_vault_test.go
Additional context used
golangci-lint
protocol/x/vault/keeper/grpc_query_vault.go
18-18: undefined: Keeper (typecheck)
61-61: undefined: Keeper (typecheck)
Biome
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/query.ts
[error] 9-9: An empty interface is equivalent to {}. (lint/suspicious/noEmptyInterface)
Safe fix: Use a type alias instead.
[error] 12-12: An empty interface is equivalent to {}. (lint/suspicious/noEmptyInterface)
Safe fix: Use a type alias instead.
Additional comments not posted (5)
protocol/x/vault/keeper/grpc_query_vault.go (2)
12-12: Ensure thedtypespackage is correctly imported and used, as it's crucial for handling the new data types.
55-56: The conversion of equity and inventory usingdtypes.NewIntFromBigIntaligns with the PR's goal to handle potentially negative values correctly.indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/query.ts (3)
40-41: The update ofequityandinventoryfields fromLongtoUint8Arrayin bothQueryVaultResponseandQueryVaultResponseSDKTypeinterfaces is correctly implemented to handle the new data types.Also applies to: 49-50
252-253: The initialization ofequityandinventoryasnew Uint8Array()in thecreateBaseQueryVaultResponseandfromPartialfunctions correctly reflects the changes in the data type.Also applies to: 325-326
268-273: The encoding and decoding logic forequityandinventoryfields has been updated to handleUint8Array, ensuring correct serialization and deserialization.Also applies to: 301-305
|
@Mergifyio backport release/protocol/v5.x |
✅ Backports have been created
|
…y (backport #1688) (#1693) Co-authored-by: Tian <[email protected]>
Changelist
use serializable int for equity and inventory in vault query as equity and inventory can be negative
Test Plan
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.Summary by CodeRabbit
Refactor
equityandinventoryfields to improve performance and reliability.Tests
equityandinventory.These changes ensure more efficient data processing and robustness in various financial computations.