-
Notifications
You must be signed in to change notification settings - Fork 115
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-461] define MsgWithdrawFromVault
proto
#1815
Conversation
WalkthroughThe overall changes entail refining the functionality for asset management in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client
participant VaultService
User->>Client: Initiate Withdraw Request
Client->>VaultService: Send MsgWithdrawFromVault
VaultService-->>Client: Return MsgWithdrawFromVaultResponse
Client-->>User: Display Withdrawal Response
Poem
Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
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 (
|
proto/dydxprotocol/vault/tx.proto
Outdated
dydxprotocol.subaccounts.SubaccountId subaccount_id = 2; | ||
|
||
// Number of shares to withdraw. | ||
uint64 shares = 3; |
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.
the original spec calls for quote_quantums
but i think it makes more sense to specify the # of shares. wdyt?
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.
offline conclusion: decided to go w specifying USDC (slack thread)
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.
offline conclusion: decided to go w specifying shares
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 (1)
- proto/dydxprotocol/vault/tx.proto (2 hunks)
Additional comments not posted (4)
proto/dydxprotocol/vault/tx.proto (4)
21-21
: Updated description enhances clarity.The updated comment for
MsgDepositToVault
improves clarity about the operation, which is beneficial for developers interacting with this API.
42-53
: Well-defined new message for vault withdrawal.The
MsgWithdrawFromVault
is clearly defined with appropriate fields and types, aligning well with the existing message structures in this protocol.
21-21
: Verify consistency of field annotations across similar messages.The use of custom types and nullability settings here should be consistently applied across similar messages to ensure uniformity.
Verification successful
Field annotations are consistent across similar messages.
The use of the custom type
github.com/dydxprotocol/v4-chain/protocol/dtypes.SerializableInt
and the nullability setting(gogoproto.nullable) = false
is consistent across various proto messages.
- Verified consistency in multiple proto messages:
MsgDepositToVault
MsgDepositToVaultResponse
- Other related messages
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistency of field annotations in similar messages. # Test: Search for similar field annotations. Expect: Consistent use of custom types and nullability. rg --type proto "github.com/dydxprotocol/v4-chain/protocol/dtypes.SerializableInt"Length of output: 112
Script:
#!/bin/bash # Description: Verify consistency of field annotations in similar messages. # Find all .proto files and search for field annotations related to SerializableInt. fd --extension proto --exec rg "github.com/dydxprotocol/v4-chain/protocol/dtypes.SerializableInt" {}Length of output: 1850
Script:
#!/bin/bash # Description: Verify the context of field annotations in proto files for consistency. # Find all .proto files and search for the context around field annotations related to SerializableInt. fd --extension proto --exec rg -C 5 "github.com/dydxprotocol/v4-chain/protocol/dtypes.SerializableInt" {}Length of output: 7987
54-55
: Clarification needed for empty response message.The
MsgWithdrawFromVaultResponse
lacks fields, which may be intentional. Verify if this pattern is consistent with other response messages in the protocol.Verification successful
Empty response message design is consistent.
The
MsgWithdrawFromVaultResponse
message without fields aligns with the design pattern observed in other response messages in the protocol.
- Examples:
MsgDepositToVaultResponse
,MsgCreateTransferResponse
,MsgSlashValidatorResponse
, etc.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for other response messages without fields. # Test: Search for other empty response messages. Expect: Consistency in design. rg --type proto "message .*Response {}"Length of output: 165
Script:
#!/bin/bash # Description: Check for other response messages without fields. # Test: Search for other empty response messages. Expect: Consistency in design. rg --type proto "message .*Response \{\}"Length of output: 73
Script:
#!/bin/bash # Description: Check for other response messages without fields. # Find all .proto files and search for empty response messages within them. fd -e proto -x rg "message .*Response \{\}" {}Length of output: 1701
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 (1)
- proto/dydxprotocol/vault/tx.proto (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- proto/dydxprotocol/vault/tx.proto
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 (3)
- proto/dydxprotocol/vault/query.proto (1 hunks)
- proto/dydxprotocol/vault/tx.proto (2 hunks)
- proto/dydxprotocol/vault/vault.proto (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- proto/dydxprotocol/vault/tx.proto
Additional comments not posted (2)
proto/dydxprotocol/vault/vault.proto (1)
36-40
: LGTM!The
OwnerShare
message is well-defined and includes the necessary fields for representing owner shares in a vault.proto/dydxprotocol/vault/query.proto (1)
Line range hint
35-35
: LGTM!The modification to use
repeated OwnerShare owner_shares
inQueryOwnerSharesResponse
simplifies the structure and improves readability.
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
Outside diff range and nitpick comments (4)
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/vault.ts (1)
Line range hint
33-34
: Remove Redundant Switch CasesThe switch cases are redundant because the default case is already present. Remove the useless cases.
- case 0: - case "VAULT_TYPE_UNSPECIFIED": + case "VAULT_TYPE_UNSPECIFIED":- case 1: - case "VAULT_TYPE_CLOB": + case "VAULT_TYPE_CLOB":- case -1: - case "UNRECOGNIZED": + case "UNRECOGNIZED":Also applies to: 47-47
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (1)
Line range hint
38-38
: Replace Empty Interfaces with Type AliasesEmpty interfaces are equivalent to
{}
. Replace them with type aliases.- export interface MsgDepositToVaultResponse {} + export type MsgDepositToVaultResponse = {};- export interface MsgDepositToVaultResponseSDKType {} + export type MsgDepositToVaultResponseSDKType = {};- export interface MsgUpdateParamsResponse {} + export type MsgUpdateParamsResponse = {};- export interface MsgUpdateParamsResponseSDKType {} + export type MsgUpdateParamsResponseSDKType = {};Also applies to: 41-41, 128-128, 131-131
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/query.ts (2)
Line range hint
9-9
: Replace empty interface with type alias.An empty interface is equivalent to
{}
. ReplaceQueryParamsRequest
with a type alias.- export interface QueryParamsRequest {} + export type QueryParamsRequest = {};
Line range hint
12-12
: Replace empty interface with type alias.An empty interface is equivalent to
{}
. ReplaceQueryParamsRequestSDKType
with a type alias.- export interface QueryParamsRequestSDKType {} + export type QueryParamsRequestSDKType = {};
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
protocol/x/vault/types/query.pb.go
is excluded by!**/*.pb.go
protocol/x/vault/types/tx.pb.go
is excluded by!**/*.pb.go
protocol/x/vault/types/vault.pb.go
is excluded by!**/*.pb.go
Files selected for processing (6)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/genesis.ts (1 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/query.ts (3 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (4 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/vault.ts (2 hunks)
- proto/dydxprotocol/vault/query.proto (2 hunks)
- proto/dydxprotocol/vault/vault.proto (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- proto/dydxprotocol/vault/query.proto
- proto/dydxprotocol/vault/vault.proto
Additional context used
Biome
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/vault.ts
[error] 33-33: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 34-34: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 47-47: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts
[error] 38-38: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 41-41: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 128-128: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 131-131: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
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)
[error] 12-12: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
Additional comments not posted (6)
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/genesis.ts (1)
2-2
: LGTM!The import statement has been correctly updated to reflect the new location of
OwnerShare
andOwnerShareSDKType
.indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/vault.ts (2)
82-87
: NewOwnerShare
InterfaceThe new
OwnerShare
interface andOwnerShareSDKType
have been correctly defined to represent owner shares in a vault.Also applies to: 90-93
195-199
: New Functions forOwnerShare
The functions
createBaseOwnerShare
,encode
,decode
, andfromPartial
forOwnerShare
have been correctly implemented. These functions ensure proper encoding, decoding, and partial creation ofOwnerShare
objects.Also applies to: 202-246
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (2)
42-42
: NewMsgWithdrawFromVault
InterfaceThe new
MsgWithdrawFromVault
interface has been correctly defined to handle withdrawals from the vault to subaccounts. The related functions for encoding, decoding, and partial creation are implemented correctly.Also applies to: 46-84
86-108
: NewMsgWithdrawFromVaultResponse
InterfaceThe new
MsgWithdrawFromVaultResponse
interface has been correctly defined to handle the response of withdrawals from the vault. The related functions for encoding, decoding, and partial creation are implemented correctly.indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/query.ts (1)
1-1
: Update import path forOwnerShare
andOwnerShareSDKType
.The import path for
OwnerShare
andOwnerShareSDKType
has been updated to./vault
. Ensure that the new path is correct and aligns with the changes made in thevault.ts
file.
proto/dydxprotocol/vault/tx.proto
Outdated
dydxprotocol.subaccounts.SubaccountId subaccount_id = 2; | ||
|
||
// Number of shares to withdraw. | ||
uint64 shares = 3; |
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.
offline conclusion: decided to go w specifying USDC (slack thread)
@@ -82,12 +81,6 @@ message QueryOwnerSharesRequest { | |||
cosmos.base.query.v1beta1.PageRequest pagination = 3; | |||
} | |||
|
|||
// OwnerShare is a type for owner shares in a vault. | |||
message OwnerShare { |
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.
moved it to vault.proto
} | ||
|
||
// MsgWithdrawFromVaultResponse is the Msg/WithdrawFromVault response type. | ||
message MsgWithdrawFromVaultResponse { |
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.
note that this has been updated
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
Outside diff range and nitpick comments (3)
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (3)
Line range hint
38-38
: Replace empty interface with type alias.The
MsgDepositToVaultResponse
interface is empty and should be replaced with a type alias to avoid linting errors.- export interface MsgDepositToVaultResponse {} + export type MsgDepositToVaultResponse = {};
Line range hint
128-128
: Replace empty interface with type alias.The
MsgUpdateParamsResponse
interface is empty and should be replaced with a type alias to avoid linting errors.- export interface MsgUpdateParamsResponse {} + export type MsgUpdateParamsResponse = {};
Line range hint
131-131
: Replace empty interface with type alias.The
MsgUpdateParamsResponseSDKType
interface is empty and should be replaced with a type alias to avoid linting errors.- export interface MsgUpdateParamsResponseSDKType {} + export type MsgUpdateParamsResponseSDKType = {};
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 (2)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (4 hunks)
- proto/dydxprotocol/vault/tx.proto (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- proto/dydxprotocol/vault/tx.proto
Additional context used
Biome
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts
[error] 38-38: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 41-41: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 128-128: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 131-131: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
Additional comments not posted (16)
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (16)
47-63
: LGTM!The
MsgWithdrawFromVault
interface is correctly defined.
240-255
: LGTM!The
encode
function forMsgWithdrawFromVault
is correctly defined.
257-285
: LGTM!The
decode
function forMsgWithdrawFromVault
is correctly defined.
287-293
: LGTM!The
fromPartial
function forMsgWithdrawFromVault
is correctly defined.
69-84
: LGTM!The
MsgWithdrawFromVaultSDKType
interface is correctly defined.
240-255
: LGTM!The
encode
function forMsgWithdrawFromVaultSDKType
is correctly defined.
257-285
: LGTM!The
decode
function forMsgWithdrawFromVaultSDKType
is correctly defined.
287-293
: LGTM!The
fromPartial
function forMsgWithdrawFromVaultSDKType
is correctly defined.
88-97
: LGTM!The
MsgWithdrawFromVaultResponse
interface is correctly defined.
306-320
: LGTM!The
encode
function forMsgWithdrawFromVaultResponse
is correctly defined.
322-350
: LGTM!The
decode
function forMsgWithdrawFromVaultResponse
is correctly defined.
352-358
: LGTM!The
fromPartial
function forMsgWithdrawFromVaultResponse
is correctly defined.
100-108
: LGTM!The
MsgWithdrawFromVaultResponseSDKType
interface is correctly defined.
306-320
: LGTM!The
encode
function forMsgWithdrawFromVaultResponseSDKType
is correctly defined.
322-350
: LGTM!The
decode
function forMsgWithdrawFromVaultResponseSDKType
is correctly defined.
352-358
: LGTM!The
fromPartial
function forMsgWithdrawFromVaultResponseSDKType
is correctly defined.
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
Outside diff range and nitpick comments (3)
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (3)
Line range hint
38-38
: Replace empty interface with type alias.The
MsgDepositToVaultResponse
interface is empty and should be replaced with a type alias to avoid linting errors.- export interface MsgDepositToVaultResponse {} + export type MsgDepositToVaultResponse = {};
Line range hint
140-140
: Replace empty interface with type alias.The
MsgUpdateParamsResponse
interface is empty and should be replaced with a type alias to avoid linting errors.- export interface MsgUpdateParamsResponse {} + export type MsgUpdateParamsResponse = {};
Line range hint
143-143
: Replace empty interface with type alias.The
MsgUpdateParamsResponseSDKType
interface is empty and should be replaced with a type alias to avoid linting errors.- export interface MsgUpdateParamsResponseSDKType {} + export type MsgUpdateParamsResponseSDKType = {};
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 (2)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (4 hunks)
- proto/dydxprotocol/vault/tx.proto (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- proto/dydxprotocol/vault/tx.proto
Additional context used
Biome
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts
[error] 38-38: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 41-41: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 140-140: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 143-143: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
Additional comments not posted (11)
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (11)
1-1
: Import statement looks good.The necessary imports have been updated to include
NumShares
andNumSharesSDKType
.
42-69
: LGTM!The
MsgWithdrawFromVault
interface and its SDK type have been correctly defined with the necessary fields and comments.
98-120
: LGTM!The
MsgWithdrawFromVaultResponse
interface and its SDK type have been correctly defined with the necessary fields and comments.
244-250
: Base message creation function looks good.The
createBaseMsgWithdrawFromVault
function correctly initializes the base message.
252-267
: Encoding function looks good.The
encode
function correctly handles the encoding of theMsgWithdrawFromVault
message.
269-297
: Decoding function looks good.The
decode
function correctly handles the decoding of theMsgWithdrawFromVault
message.
299-305
: Partial creation function looks good.The
fromPartial
function correctly handles the creation of a partialMsgWithdrawFromVault
message.
309-315
: Base message creation function looks good.The
createBaseMsgWithdrawFromVaultResponse
function correctly initializes the base message.
317-332
: Encoding function looks good.The
encode
function correctly handles the encoding of theMsgWithdrawFromVaultResponse
message.
334-361
: Decoding function looks good.The
decode
function correctly handles the decoding of theMsgWithdrawFromVaultResponse
message.
364-370
: Partial creation function looks good.The
fromPartial
function correctly handles the creation of a partialMsgWithdrawFromVaultResponse
message.
@@ -33,6 +39,86 @@ export interface MsgDepositToVaultResponse {} | |||
/** MsgDepositToVaultResponse is the Msg/DepositToVault response type. */ | |||
|
|||
export interface MsgDepositToVaultResponseSDKType {} |
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.
Replace empty interface with type alias.
The MsgDepositToVaultResponseSDKType
interface is empty and should be replaced with a type alias to avoid linting errors.
- export interface MsgDepositToVaultResponseSDKType {}
+ export type MsgDepositToVaultResponseSDKType = {};
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 MsgDepositToVaultResponseSDKType {} | |
export type MsgDepositToVaultResponseSDKType = {}; |
Tools
Biome
[error] 41-41: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
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 (2)
protocol/x/vault/types/genesis.pb.go
is excluded by!**/*.pb.go
protocol/x/vault/types/tx.pb.go
is excluded by!**/*.pb.go
Files selected for processing (9)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.rpc.msg.ts (3 hunks)
- proto/dydxprotocol/vault/genesis.proto (1 hunks)
- proto/dydxprotocol/vault/tx.proto (2 hunks)
- protocol/app/module/interface_registry.go (1 hunks)
- protocol/app/msgs/all_msgs.go (1 hunks)
- protocol/app/msgs/normal_msgs.go (1 hunks)
- protocol/app/msgs/normal_msgs_test.go (1 hunks)
- protocol/testutil/encoding/utils.go (1 hunks)
- protocol/x/vault/keeper/msg_server_withdraw_from_vault.go (1 hunks)
Files skipped from review due to trivial changes (1)
- proto/dydxprotocol/vault/genesis.proto
Files skipped from review as they are similar to previous changes (1)
- proto/dydxprotocol/vault/tx.proto
Additional comments not posted (5)
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.rpc.msg.ts (1)
11-11
: LGTM!The method
withdrawFromVault
correctly handles the request and response for withdrawing from the vault.Also applies to: 32-36
protocol/testutil/encoding/utils.go (1)
96-96
: LGTM!The
MsgWithdrawFromVault
message is correctly added to the list of messages to register.protocol/app/module/interface_registry.go (1)
98-100
: LGTM!The
MsgWithdrawFromVault
message is correctly added to the list of custom get signers.protocol/app/msgs/normal_msgs.go (1)
239-242
: LGTM!The newly added messages for
MsgWithdrawFromVault
andMsgWithdrawFromVaultResponse
are correctly included in theNormalMsgsDydxCustom
map.protocol/app/msgs/all_msgs.go (1)
242-245
: LGTM!The newly added messages for
MsgWithdrawFromVault
andMsgWithdrawFromVaultResponse
are correctly included in theAllTypeMessages
map.
"/dydxprotocol.vault.MsgWithdrawFromVault", | ||
"/dydxprotocol.vault.MsgWithdrawFromVaultResponse", |
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 assertions for new message types.
The test function TestNormalMsgs_Key
should include assertions for the newly added message types to ensure they are correctly handled.
+ "/dydxprotocol.vault.MsgWithdrawFromVault",
+ "/dydxprotocol.vault.MsgWithdrawFromVaultResponse",
Committable suggestion was skipped due to low confidence.
// TODO: implement | ||
func (k msgServer) WithdrawFromVault( | ||
goCtx context.Context, | ||
msg *types.MsgWithdrawFromVault, | ||
) (*types.MsgWithdrawFromVaultResponse, error) { | ||
return &types.MsgWithdrawFromVaultResponse{}, nil |
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.
Implement the WithdrawFromVault
function.
The function currently returns an empty response. You should implement the logic to handle the withdrawal from the vault.
Do you want me to help implement the withdrawal logic or open a GitHub issue to track this task?
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 (2)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (4 hunks)
- proto/dydxprotocol/vault/tx.proto (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- proto/dydxprotocol/vault/tx.proto
Additional context used
Biome
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts
[error] 38-38: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 41-41: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 142-142: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 145-145: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
Additional comments not posted (8)
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (8)
47-64
: Verify the necessity of optional fields.The fields
vaultId
,subaccountId
, andshares
are marked as optional. Ensure that these fields can indeed be optional in all cases.
70-86
: Verify the necessity of optional fields.The fields
vault_id
,subaccount_id
, andshares
are marked as optional. Ensure that these fields can indeed be optional in all cases.
90-104
: Verify the necessity of optional fields.The fields
redeemedShares
,remainingShares
, andtotalVaultShares
are marked as optional. Ensure that these fields can indeed be optional in all cases.
108-122
: Verify the necessity of optional fields.The fields
redeemed_shares
,remaining_shares
, andtotal_vault_shares
are marked as optional. Ensure that these fields can indeed be optional in all cases.
246-252
: LGTM!The function correctly initializes the
MsgWithdrawFromVault
interface with default values.
254-309
: LGTM!The function correctly handles encoding, decoding, and partial message creation for
MsgWithdrawFromVault
.
311-319
: LGTM!The function correctly initializes the
MsgWithdrawFromVaultResponse
interface with default values.
321-394
: LGTM!The function correctly handles encoding, decoding, and partial message creation for
MsgWithdrawFromVaultResponse
.
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 (2)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (4 hunks)
- proto/dydxprotocol/vault/tx.proto (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- proto/dydxprotocol/vault/tx.proto
Additional context used
Biome
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts
[error] 38-38: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 41-41: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 142-142: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 145-145: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
Additional comments not posted (6)
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (6)
47-64
: LGTM! New message typeMsgWithdrawFromVault
.The new message type
MsgWithdrawFromVault
is correctly defined with propertiesvaultId
,subaccountId
, andshares
.
70-87
: LGTM! New SDK typeMsgWithdrawFromVaultSDKType
.The new SDK type
MsgWithdrawFromVaultSDKType
is correctly defined with propertiesvault_id
,subaccount_id
, andshares
.
90-105
: LGTM! New message typeMsgWithdrawFromVaultResponse
.The new message type
MsgWithdrawFromVaultResponse
is correctly defined with propertiesredeemedShares
,withdrawnQuoteQuantums
,remainingShares
,totalVaultShares
, andtotalVaultEquity
.
108-123
: LGTM! New SDK typeMsgWithdrawFromVaultResponseSDKType
.The new SDK type
MsgWithdrawFromVaultResponseSDKType
is correctly defined with propertiesredeemed_shares
,withdrawn_quote_quantums
,remaining_shares
,total_vault_shares
, andtotal_vault_equity
.
254-309
: LGTM! Encoding/decoding functions forMsgWithdrawFromVault
.The encoding/decoding functions for
MsgWithdrawFromVault
are correctly defined and follow the structure of other similar functions in the file.
321-394
: LGTM! Encoding/decoding functions forMsgWithdrawFromVaultResponse
.The encoding/decoding functions for
MsgWithdrawFromVaultResponse
are correctly defined and follow the structure of other similar functions in the 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 (2)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (4 hunks)
- proto/dydxprotocol/vault/tx.proto (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- proto/dydxprotocol/vault/tx.proto
Additional context used
Biome
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts
[error] 38-38: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 41-41: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 142-142: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 145-145: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
Additional comments not posted (8)
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (8)
1-1
: Import statement is correct.The import statement includes the necessary types from the
vault
module.
47-64
: LGTM! TheMsgWithdrawFromVault
interface is well-defined.The interface aligns with the purpose of withdrawing from a vault.
70-87
: LGTM! TheMsgWithdrawFromVaultSDKType
interface is well-defined.The interface aligns with the purpose of withdrawing from a vault.
90-104
: LGTM! TheMsgWithdrawFromVaultResponse
interface is well-defined.The interface aligns with the purpose of providing a response for withdrawing from a vault.
108-122
: LGTM! TheMsgWithdrawFromVaultResponseSDKType
interface is well-defined.The interface aligns with the purpose of providing a response for withdrawing from a vault.
246-252
: LGTM! ThecreateBaseMsgWithdrawFromVault
function is well-defined.The function initializes the base message correctly.
254-309
: LGTM! The encoding and decoding functions forMsgWithdrawFromVault
are well-defined.The functions handle the serialization and deserialization correctly.
321-394
: LGTM! The encoding and decoding functions forMsgWithdrawFromVaultResponse
are well-defined.The functions handle the serialization and deserialization correctly.
Changelist
MsgWithdrawFromVault
protomsg_server
implmake proto-gen
andcd indexer/packages/v4-protos && npm run build:proto
Test Plan
n/a
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
MsgWithdrawFromVault
andMsgWithdrawFromVaultResponse
.Documentation
Chores