[CP-572][exchange] added MsgSetDelegationTransferReceivers type #327
Conversation
WalkthroughAdds a new MsgSetDelegationTransferReceivers message type with SDK methods and interface assertion, and wires a corresponding RPC in the exchange Msg service via protobuf, introducing request/response messages for setting delegation transfer receivers. Minor formatting adjustments elsewhere without behavioral impact. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MsgService as Msg Service (Exchange)
participant Handler
participant Keeper
participant State
Client->>MsgService: SetDelegationTransferReceivers(request)
MsgService->>Handler: ValidateBasic(sender, receivers)
Handler->>Keeper: Set delegation transfer receivers
Keeper->>State: Update receivers list for sender
State-->>Keeper: OK
Keeper-->>Handler: Success
Handler-->>MsgService: MsgSetDelegationTransferReceiversResponse
MsgService-->>Client: Response
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
proto/injective/exchange/v2/tx.proto (1)
1524-1535: Consider aligning message options with neighboring messages (gogo options for consistency)Most neighboring messages set gogoproto options to avoid generating equality methods and getters. While optional, aligning improves codegen consistency across the SDK.
Suggested proto tweaks:
message MsgSetDelegationTransferReceivers { option (amino.name) = "exchange/MsgSetDelegationTransferReceivers"; option (cosmos.msg.v1.signer) = "sender"; + option (gogoproto.equal) = false; + option (gogoproto.goproto_getters) = false; // the sender's Injective address (must be exchange admin) string sender = 1; // list of receiver addresses to set as delegation transfer receivers repeated string receivers = 2; }Optionally, to annotate address scalars (not currently used elsewhere in this file for sender fields, so treat as optional):
- sender: [cosmos_proto.scalar] = "cosmos.AddressString"
- receivers: not typically annotated, but could be if you want stricter typing.
If you’d like, I can raise a follow-up PR to uniformly apply gogoproto options and address annotations across all new message definitions for consistency.
chain/exchange/types/v2/msgs.go (2)
2610-2612: Use a msg type constant for consistency with the rest of the fileAll other messages return constants from the Type() method. Define and use a constant to keep naming centralized.
Apply within this hunk:
-func (msg *MsgSetDelegationTransferReceivers) Type() string { return "setDelegationTransferReceivers" } +func (msg *MsgSetDelegationTransferReceivers) Type() string { return TypeMsgSetDelegationTransferReceivers }And add this constant alongside the other TypeMsg* constants (outside this hunk):
// exchange message types const ( // ... existing constants TypeMsgSetDelegationTransferReceivers = "setDelegationTransferReceivers" )
2640-2640: Optional: fold interface assertion into the existing block for consistencyYou already have a big var block asserting sdk.Msg implementations at Lines 21–70. Consider adding MsgSetDelegationTransferReceivers there and removing this trailing line for consistency.
If you choose to do that, remove this line and add:
var ( // ... existing assertions _ sdk.Msg = &MsgSetDelegationTransferReceivers{} )Do you want me to open a small follow-up PR to tidy up these consistency changes?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
chain/exchange/types/v2/tx.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (2)
chain/exchange/types/v2/msgs.go(1 hunks)proto/injective/exchange/v2/tx.proto(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
chain/exchange/types/v2/msgs.go (4)
chain/exchange/types/v2/tx.pb.go (3)
MsgSetDelegationTransferReceivers(4934-4939)MsgSetDelegationTransferReceivers(4943-4943)MsgSetDelegationTransferReceivers(4944-4946)chain/exchange/types/msgs.go (1)
RouterKey(18-18)typeddata/typed_data.go (1)
Type(69-72)chain/exchange/types/errors.go (1)
ErrInvalidAddress(91-91)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: run-tests
- GitHub Check: lint
🔇 Additional comments (3)
proto/injective/exchange/v2/tx.proto (3)
222-223: RPC addition looks correct and consistent with existing Msg service methodsThe new RPC is properly wired with request/response types and uses the expected naming. No concerns.
1522-1523: No-op formatting changeThis appears to be a whitespace-only adjustment around the response message. No behavioral impact.
1536-1538: Response type definition is fineEmpty response is consistent with similar RPCs in this service.
| if _, err := sdk.AccAddressFromBech32(msg.Sender); err != nil { | ||
| return errors.Wrap(sdkerrors.ErrInvalidAddress, msg.Sender) | ||
| } | ||
|
|
||
| if len(msg.Receivers) == 0 { | ||
| return errors.Wrap(sdkerrors.ErrInvalidRequest, "receivers list cannot be empty") | ||
| } | ||
|
|
||
| for _, receiver := range msg.Receivers { | ||
| if _, err := sdk.AccAddressFromBech32(receiver); err != nil { | ||
| return errors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid receiver address: %s", receiver) | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden validation: prefer domain error and guard against duplicate receivers
- Prefer using a domain error for invalid/empty fields (ErrBadField is used elsewhere in this file for similar cases).
- Prevent duplicates in the receivers list to avoid redundant state writes and ambiguity.
Apply:
func (msg *MsgSetDelegationTransferReceivers) ValidateBasic() error {
if _, err := sdk.AccAddressFromBech32(msg.Sender); err != nil {
return errors.Wrap(sdkerrors.ErrInvalidAddress, msg.Sender)
}
- if len(msg.Receivers) == 0 {
- return errors.Wrap(sdkerrors.ErrInvalidRequest, "receivers list cannot be empty")
- }
+ if len(msg.Receivers) == 0 {
+ return errors.Wrap(types.ErrBadField, "receivers list cannot be empty")
+ }
- for _, receiver := range msg.Receivers {
+ seen := make(map[string]struct{}, len(msg.Receivers))
+ for _, receiver := range msg.Receivers {
if _, err := sdk.AccAddressFromBech32(receiver); err != nil {
return errors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid receiver address: %s", receiver)
}
+ if _, ok := seen[receiver]; ok {
+ return errors.Wrap(types.ErrBadField, "duplicate receiver addresses found")
+ }
+ seen[receiver] = struct{}{}
}
return nil
}📝 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.
| if _, err := sdk.AccAddressFromBech32(msg.Sender); err != nil { | |
| return errors.Wrap(sdkerrors.ErrInvalidAddress, msg.Sender) | |
| } | |
| if len(msg.Receivers) == 0 { | |
| return errors.Wrap(sdkerrors.ErrInvalidRequest, "receivers list cannot be empty") | |
| } | |
| for _, receiver := range msg.Receivers { | |
| if _, err := sdk.AccAddressFromBech32(receiver); err != nil { | |
| return errors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid receiver address: %s", receiver) | |
| } | |
| } | |
| return nil | |
| } | |
| func (msg *MsgSetDelegationTransferReceivers) ValidateBasic() error { | |
| if _, err := sdk.AccAddressFromBech32(msg.Sender); err != nil { | |
| return errors.Wrap(sdkerrors.ErrInvalidAddress, msg.Sender) | |
| } | |
| if len(msg.Receivers) == 0 { | |
| return errors.Wrap(types.ErrBadField, "receivers list cannot be empty") | |
| } | |
| seen := make(map[string]struct{}, len(msg.Receivers)) | |
| for _, receiver := range msg.Receivers { | |
| if _, err := sdk.AccAddressFromBech32(receiver); err != nil { | |
| return errors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid receiver address: %s", receiver) | |
| } | |
| if _, ok := seen[receiver]; ok { | |
| return errors.Wrap(types.ErrBadField, "duplicate receiver addresses found") | |
| } | |
| seen[receiver] = struct{}{} | |
| } | |
| return nil | |
| } |
🤖 Prompt for AI Agents
In chain/exchange/types/v2/msgs.go around lines 2613 to 2628, the validation
currently returns sdkerrors.ErrInvalidAddress and sdkerrors.ErrInvalidRequest;
change these to use the domain error sdkerrors.ErrBadField for invalid/empty
fields (match the rest of the file) by wrapping validation failures with
ErrBadField and an explanatory message for sender and receivers; additionally,
add a duplicate-check for msg.Receivers (use a map[string]struct{} while
iterating) and if a duplicate is found return an ErrBadField indicating
duplicate receiver addresses; keep address parsing logic but return ErrBadField
on parse errors with the offending address in the message.
| func (msg *MsgSetDelegationTransferReceivers) GetSignBytes() []byte { | ||
| bz, _ := json.Marshal(msg) | ||
| return sdk.MustSortJSON(bz) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use ModuleCdc for sign bytes to match the rest of the codebase
Other messages use the module codec and MustMarshalJSON for sign bytes. This ensures consistent JSON encoding across messages.
func (msg *MsgSetDelegationTransferReceivers) GetSignBytes() []byte {
- bz, _ := json.Marshal(msg)
- return sdk.MustSortJSON(bz)
+ return sdk.MustSortJSON(types.ModuleCdc.MustMarshalJSON(msg))
}📝 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.
| func (msg *MsgSetDelegationTransferReceivers) GetSignBytes() []byte { | |
| bz, _ := json.Marshal(msg) | |
| return sdk.MustSortJSON(bz) | |
| } | |
| func (msg *MsgSetDelegationTransferReceivers) GetSignBytes() []byte { | |
| return sdk.MustSortJSON(types.ModuleCdc.MustMarshalJSON(msg)) | |
| } |
🤖 Prompt for AI Agents
In chain/exchange/types/v2/msgs.go around lines 2630 to 2633, the GetSignBytes
method uses json.Marshal and sdk.MustSortJSON; change it to use the module codec
and its MustMarshalJSON to match other messages: replace json.Marshal(msg) with
ModuleCdc.MustMarshalJSON(msg) (or the package-level module codec used across
the codebase), then pass that result to sdk.MustSortJSON as before so sign bytes
are produced with the module codec's deterministic encoding.
| func (msg *MsgSetDelegationTransferReceivers) GetSigners() []sdk.AccAddress { | ||
| sender, _ := sdk.AccAddressFromBech32(msg.Sender) | ||
| return []sdk.AccAddress{sender} | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t ignore address parsing errors in GetSigners
Every other GetSigners implementation panics on invalid bech32, which is preferred over silently ignoring errors.
func (msg *MsgSetDelegationTransferReceivers) GetSigners() []sdk.AccAddress {
- sender, _ := sdk.AccAddressFromBech32(msg.Sender)
- return []sdk.AccAddress{sender}
+ sender, err := sdk.AccAddressFromBech32(msg.Sender)
+ if err != nil {
+ panic(err)
+ }
+ return []sdk.AccAddress{sender}
}📝 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.
| func (msg *MsgSetDelegationTransferReceivers) GetSigners() []sdk.AccAddress { | |
| sender, _ := sdk.AccAddressFromBech32(msg.Sender) | |
| return []sdk.AccAddress{sender} | |
| } | |
| func (msg *MsgSetDelegationTransferReceivers) GetSigners() []sdk.AccAddress { | |
| sender, err := sdk.AccAddressFromBech32(msg.Sender) | |
| if err != nil { | |
| panic(err) | |
| } | |
| return []sdk.AccAddress{sender} | |
| } |
🤖 Prompt for AI Agents
In chain/exchange/types/v2/msgs.go around lines 2635 to 2638, the GetSigners
method currently ignores the error returned by sdk.AccAddressFromBech32; update
it to check the error and panic on invalid bech32 like the other GetSigners
implementations. Specifically, call sdk.AccAddressFromBech32(msg.Sender), if err
!= nil panic with a clear message (e.g. fmt.Sprintf or sdk errors) indicating
invalid bech32 address, otherwise return the parsed sender in the slice.
maxim-inj
changed the title
|
Closing this PR because the changes have been applied in #328 |
Changes needed to support a new type from
v1.16.3testnet release.Summary by CodeRabbit