-
Notifications
You must be signed in to change notification settings - Fork 122
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
return mintedShares in vault deposit keeper method #2139
Conversation
WalkthroughThe changes involve enhancements to the deposit functionality within the megavault system, focusing on the addition of minted shares in responses. Key updates include modifications to interfaces, methods, and protocol files to support this new property. The deposit process is streamlined by consolidating operations, improving error handling, and ensuring that relevant information about shares is consistently provided across different components. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MsgServer
participant Keeper
participant Vault
User->>MsgServer: DepositToMegavault(msg)
MsgServer->>Keeper: DepositToMegavault(ctx, fromSubaccount, quoteQuantums)
Keeper->>Vault: MintShares(ctx, owner, quantumsToDeposit)
Vault-->>Keeper: Return mintedShares
Keeper-->>MsgServer: Return mintedShares, error
MsgServer-->>User: MsgDepositToMegavaultResponse(mintedShares)
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 Configuration 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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
protocol/x/vault/types/tx.pb.go
is excluded by!**/*.pb.go
Files selected for processing (5)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (5 hunks)
- proto/dydxprotocol/vault/tx.proto (2 hunks)
- protocol/x/vault/keeper/deposit.go (5 hunks)
- protocol/x/vault/keeper/deposit_test.go (2 hunks)
- protocol/x/vault/keeper/msg_server_deposit_to_megavault.go (2 hunks)
Additional comments not posted (11)
protocol/x/vault/keeper/msg_server_deposit_to_megavault.go (1)
18-29
: LGTM! Verify the usage of the updated function.The refactor simplifies the deposit process and improves maintainability by consolidating operations into a single call and returning minted shares directly.
Ensure that the updated function is correctly integrated and used throughout the codebase.
Run the following script to verify the function usage:
Verification successful
Function Integration Verified Successfully
The
DepositToMegavault
function is correctly integrated and used within the codebase, specifically inmsg_server_deposit_to_megavault.go
. The function call aligns with the new logic, confirming proper usage.
protocol/x/vault/keeper/msg_server_deposit_to_megavault.go
: Correct usage ofDepositToMegavault
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `DepositToMegavault` match the new logic. # Test: Search for the function usage. Expect: Only occurrences of the new logic. rg --type go -A 5 $'DepositToMegavault'Length of output: 31609
Script:
#!/bin/bash # Description: Search for direct invocations of the `DepositToMegavault` function in the keeper. # Test: Search for the function usage in the keeper. rg --type go -A 5 'DepositToMegavault('Length of output: 130
Script:
#!/bin/bash # Description: Search for direct invocations of the `DepositToMegavault` function in the keeper. # Test: Search for the function usage in the keeper with corrected regex. rg --type go -A 5 'DepositToMegavault\('Length of output: 6274
proto/dydxprotocol/vault/tx.proto (1)
48-51
: LGTM!The addition of the
minted_shares
field enhances the response by providing detailed feedback on share issuance.protocol/x/vault/keeper/deposit.go (2)
13-48
: LGTM!The
DepositToMegavault
function is well-structured, consolidating the deposit process and ensuring accurate share calculations.
Line range hint
49-133
: LGTM!The
MintShares
function now returns minted shares and improves error handling, enhancing its utility.protocol/x/vault/keeper/deposit_test.go (1)
Line range hint
187-213
: LGTM! Validate the test coverage.The changes improve the robustness of the test by capturing and validating
mintedShares
. Ensure that all edge cases are covered.Consider running the test suite to verify the coverage and correctness of the tests.
Run the following script to verify the test coverage:
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (6)
33-36
: LGTM! ThemintedShares
property is a valuable addition.The inclusion of the
mintedShares
property enhances the response by providing additional information about the transaction.
39-42
: LGTM! Theminted_shares
property ensures consistency.The inclusion of the
minted_shares
property in the SDK type maintains consistency with the standard type.
162-164
: LGTM! Initialization aligns with the optional property.The
mintedShares
property is correctly initialized toundefined
, reflecting its optional nature.
168-171
: LGTM! Encoding logic is correctly updated.The
encode
function now properly handles themintedShares
field, ensuring it is encoded when defined.
185-187
: LGTM! Decoding logic is correctly updated.The
decode
function now properly reconstructs themintedShares
field from the input data.
198-200
: LGTM!fromPartial
function is correctly updated.The
fromPartial
function now properly accommodates themintedShares
property, ensuring accurate transformation of partial objects.
Changelist
mintedShares
Test Plan
MintShares
DepositToMegavault
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
mintedShares
in deposit response, providing additional details on shares minted from deposits.DepositToMegavault
method, streamlining the deposit process and enhancing user interaction with megavault transactions.Bug Fixes
Tests
MintShares
to validate minted shares alongside error checking.