[CHORE] upgrade protos v1.16.3 #328
Conversation
…1.16.69. Added support for MsgSetDelegationTransferReceivers
WalkthroughIntroduces a new “SetDelegationTransferReceivers” message and RPC in the exchange module, adds a setter to StakingKeeper, updates public chain message list, and extends Params with a human_readable_upgrade_block_height field. Adjusts a Peggy claim hash component, bumps Makefile clone versions, and refines documentation/comments for funding fields. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MsgService as Exchange Msg Service
participant Handler as Exchange Msg Handler
participant Keeper
participant StakingKeeper
Client->>MsgService: MsgSetDelegationTransferReceivers{sender, receivers}
MsgService->>Handler: route(Type="setDelegationTransferReceivers")
Handler->>Keeper: validate sender, receivers
Keeper->>StakingKeeper: SetDelegationTransferReceiver(ctx, receiver) (for each)
StakingKeeper-->>Keeper: state updated
Keeper-->>Handler: ok
Handler-->>MsgService: MsgSetDelegationTransferReceiversResponse{}
MsgService-->>Client: response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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
|
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades protocol buffer definitions to align with core v1.16.3 and indexer v1.16.69, adding support for MsgSetDelegationTransferReceivers functionality and improving documentation clarity.
- Added MsgSetDelegationTransferReceivers message for setting delegation transfer receivers
- Updated PerpetualMarketFunding cumulative_price field documentation for clarity
- Added human_readable_upgrade_block_height parameter to exchange module
- Fixed token contract usage in peggy module claim hash calculation
Reviewed Changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| proto/injective/exchange/v2/tx.proto | Added new MsgSetDelegationTransferReceivers RPC method and message definitions |
| proto/injective/exchange/v2/market.proto | Updated cumulative_price field documentation to clarify it represents time-integral of perp premium |
| proto/injective/exchange/v2/exchange.proto | Added human_readable_upgrade_block_height parameter for tracking upgrade timing |
| proto/injective/exchange/v1beta1/exchange.proto | Updated cumulative_price documentation for consistency with v2 |
| injective_data/chain_messages_list.json | Added MsgSetDelegationTransferReceivers to supported message types |
| chain/peggy/types/msgs.go | Fixed ClaimHash to use BlockHeight instead of duplicate TokenContract |
| chain/exchange/types/v2/tx.pb.go | Generated code for new MsgSetDelegationTransferReceivers functionality |
| chain/exchange/types/v2/msgs.go | Added validation and interface implementations for MsgSetDelegationTransferReceivers |
| chain/exchange/types/v2/market.pb.go | Updated generated code with improved cumulative_price documentation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![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 (10)
Makefile (2)
4-4: Bump to indexer v1.16.69 looks good; consider parameterizing tags for easier maintenanceHard-coding the tag makes future bumps noisier. Suggest switching to a Make variable so downstream automation can override without editing the rule.
Apply this within the current line:
-git clone https://github.com/InjectiveLabs/injective-indexer.git -b v1.16.69 --depth 1 --single-branch +git clone https://github.com/InjectiveLabs/injective-indexer.git -b $(INDEXER_TAG) --depth 1 --single-branchAdd these definitions near the top of the Makefile to keep defaults aligned with this PR:
# Version pins (override via environment if needed) INDEXER_TAG ?= v1.16.69 CORE_TAG ?= v1.16.3
7-7: Core bump to v1.16.3 is aligned with PR; mirror the parameterization for consistencySame rationale as above. It streamlines future upgrades and allows CI overrides.
Apply this within the current line:
-git clone https://github.com/InjectiveLabs/injective-core.git -b v1.16.3 --depth 1 --single-branch +git clone https://github.com/InjectiveLabs/injective-core.git -b $(CORE_TAG) --depth 1 --single-branchproto/injective/exchange/v2/exchange.proto (1)
197-199: New Params field human_readable_upgrade_block_height — add a short note about sentinel valueMinor doc tweak to clarify behavior when unset.
- // human_readable_upgrade_block_height defines the block height at which the - // human readable upgrade took place + // human_readable_upgrade_block_height defines the block height at which the + // human readable upgrade took place. Set to 0 if not applicable.proto/injective/exchange/v1beta1/exchange.proto (1)
408-411: Comment typography nit: replace curly apostropheAvoid curly quotes in source comments to keep diffs and tooling consistent across locales.
- // used to compute the interval’s average premium for funding + // used to compute the interval's average premium for fundingproto/injective/exchange/v2/market.proto (1)
333-335: Comment typography nit: replace curly apostropheSame as v1beta1: prefer straight ASCII apostrophes.
- // used to compute the interval’s average premium for funding + // used to compute the interval's average premium for fundingchain/exchange/types/v2/msgs.go (3)
2610-2612: Use a module-level type constant for Type() to stay consistent with other msgsMost msgs in this file return a named constant (e.g., TypeMsgUpdateParams). Returning a raw string here is inconsistent and brittle.
Apply inside the changed range:
-func (msg *MsgSetDelegationTransferReceivers) Type() string { return "setDelegationTransferReceivers" } +func (msg *MsgSetDelegationTransferReceivers) Type() string { return TypeMsgSetDelegationTransferReceivers }Add this constant alongside the other message type constants (outside the changed block):
// exchange message types const ( // ... existing constants ... TypeMsgSetDelegationTransferReceivers = "setDelegationTransferReceivers" )
2621-2625: Validate and dedupe receivers to prevent duplicatesAs written, duplicate receiver addresses are allowed. If duplicates are unintended, reject or de-duplicate to avoid redundant state writes and ambiguous UX.
If duplicates are allowed by design, please confirm and we can skip this.
Suggested change:
- for _, receiver := range msg.Receivers { + seen := make(map[string]struct{}, len(msg.Receivers)) + for _, receiver := range msg.Receivers { + if _, ok := seen[receiver]; ok { + return errors.Wrapf(sdkerrors.ErrInvalidRequest, "duplicate receiver address: %s", receiver) + } + seen[receiver] = struct{}{} if _, err := sdk.AccAddressFromBech32(receiver); err != nil { return errors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid receiver address: %s", receiver) } }
2613-2628: Edge-case: Consider bounding the receivers list sizeDepending on block gas/tx size policies, an unbounded receivers list could be abused. Consider enforcing a sane upper bound (module param or a conservative constant).
If there’s already a max enforced in the handler/keeper, no change needed. Otherwise, I can propose a minimal check here.
chain/exchange/types/expected_keepers.go (1)
70-71: Consider returning an error and aligning naming/plurality with the MsgThe interface method signature:
- No error return: callers can’t distinguish failures (e.g., store write errors, permission checks). Most keeper mutators return error.
- Name is singular (SetDelegationTransferReceiver) while the Msg sets a list of receivers; ensure the handler semantics are intentional.
Recommendations:
- Prefer
SetDelegationTransferReceivers(ctx context.Context, receivers []sdk.AccAddress) erroror, if intentionally single, returnerrorand document that the msg applies multiple calls.Would you like me to draft the interface and a possible adapter in the handler that batches individual calls with proper rollback semantics?
proto/injective/exchange/v2/tx.proto (1)
1524-1534: Optional: Add address annotations and clarify constraints in protoFor stronger typing and client generation consistency, you can annotate address fields with cosmos address scalars, as seen elsewhere (e.g., MsgUpdateParams.authority). Also, if receivers must be unique, consider documenting that here.
Example:
// MsgSetDelegationTransferReceivers defines a message for setting delegation // transfer receivers message MsgSetDelegationTransferReceivers { option (amino.name) = "exchange/MsgSetDelegationTransferReceivers"; option (cosmos.msg.v1.signer) = "sender"; // the sender's Injective address (must be exchange admin) string sender = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"]; // list of receiver addresses to set as delegation transfer receivers (must be unique) repeated string receivers = 2; // optionally annotate with cosmos.AddressString per element if desired }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
⛔ Files ignored due to path filters (6)
chain/exchange/types/exchange.pb.gois excluded by!**/*.pb.gochain/exchange/types/v2/exchange.pb.gois excluded by!**/*.pb.gochain/exchange/types/v2/market.pb.gois excluded by!**/*.pb.gochain/exchange/types/v2/tx.pb.gois excluded by!**/*.pb.goexchange/derivative_exchange_rpc/pb/goadesign_goagen_injective_derivative_exchange_rpc.pb.gois excluded by!**/*.pb.goexchange/spot_exchange_rpc/pb/goadesign_goagen_injective_spot_exchange_rpc.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (9)
Makefile(1 hunks)chain/exchange/types/expected_keepers.go(1 hunks)chain/exchange/types/v2/msgs.go(1 hunks)chain/peggy/types/msgs.go(1 hunks)injective_data/chain_messages_list.json(1 hunks)proto/injective/exchange/v1beta1/exchange.proto(1 hunks)proto/injective/exchange/v2/exchange.proto(1 hunks)proto/injective/exchange/v2/market.proto(1 hunks)proto/injective/exchange/v2/tx.proto(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
chain/exchange/types/v2/msgs.go (3)
chain/exchange/types/v2/tx.pb.go (3)
MsgSetDelegationTransferReceivers(4933-4938)MsgSetDelegationTransferReceivers(4942-4942)MsgSetDelegationTransferReceivers(4943-4945)chain/exchange/types/msgs.go (1)
RouterKey(18-18)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: lint
- GitHub Check: run-tests
🔇 Additional comments (7)
chain/peggy/types/msgs.go (1)
395-396: Fix ClaimHash composition for MsgWithdrawClaim (include BlockHeight instead of duplicating TokenContract) — LGTMThis aligns the hash path with unique event identity (tokenContract/batchNonce/eventNonce/blockHeight) and removes the prior duplication. Good security-sensitive fix.
chain/exchange/types/v2/msgs.go (1)
2640-2640: LGTM: Interface conformance declarationCompile-time assertion is present and correct.
injective_data/chain_messages_list.json (1)
96-97: LGTM: Message exposed in the public chain messages listThe new MsgSetDelegationTransferReceivers entry is correctly added alongside other v2 exchange messages.
proto/injective/exchange/v2/tx.proto (3)
222-224: LGTM: New RPC added to Msg serviceThe SetDelegationTransferReceivers RPC is correctly declared and returns the corresponding response type.
1522-1523: LGTM: Response message remains empty by designNo issues with keeping MsgAtomicMarketOrderFeeMultiplierScheduleResponse empty; unrelated whitespace-only change here is harmless.
1536-1538: LGTM: Empty response type is fineThe response message is appropriately empty for a setter-style RPC.
chain/exchange/types/expected_keepers.go (1)
67-71: Cross-module compatibility check
I ran a search and only found the newSetDelegationTransferReceivermethod inchain/exchange/types/expected_keepers.go. Please confirm:
- Staking module: that
x/staking.Keeper(or any other realStakingKeeperimplementation) has been updated to define and implementSetDelegationTransferReceiver.- App wiring: that your application setup (e.g. in
app.goor module constructor) passes aStakingKeeperwith the new method into the exchange module so everything still compiles.
| func (msg *MsgSetDelegationTransferReceivers) Route() string { return RouterKey } | ||
| func (msg *MsgSetDelegationTransferReceivers) Type() string { return "setDelegationTransferReceivers" } | ||
| 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") | ||
| } | ||
|
|
||
| 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) GetSignBytes() []byte { | ||
| bz, _ := json.Marshal(msg) | ||
| return sdk.MustSortJSON(bz) | ||
| } | ||
|
|
||
| func (msg *MsgSetDelegationTransferReceivers) GetSigners() []sdk.AccAddress { | ||
| sender, _ := sdk.AccAddressFromBech32(msg.Sender) | ||
| return []sdk.AccAddress{sender} | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure Msg server and keeper support are implemented
This PR wires the Msg type and proto, but the Msg server handler and keeper logic must exist to effect changes and call StakingKeeper.SetDelegationTransferReceiver for each receiver.
I can help audit and scaffold the Msg server if needed. To locate the handler, run:
🏁 Script executed:
#!/bin/bash
# Verify msg server implementation exists for SetDelegationTransferReceivers
rg -n --context 3 'SetDelegationTransferReceivers\\(.*MsgSetDelegationTransferReceivers'
rg -n --context 3 'MsgSetDelegationTransferReceivers'Length of output: 23031
Implement Msg server and keeper handler for SetDelegationTransferReceivers
The protobuf, Go message type, and client stub are in place, but there’s currently no gRPC service method or keeper logic to process MsgSetDelegationTransferReceivers. You’ll need to:
- Add the
SetDelegationTransferReceiversRPC handler in your keeper (e.g. inchain/exchange/keeper/msg_server.go) that implements the generatedMsgServerinterface. - In that handler:
- Convert the incoming
senderto ansdk.AccAddress. - Loop over
msg.Receivers, converting each tosdk.AccAddressand calling
k.StakingKeeper.SetDelegationTransferReceiver(ctx, sender, receiver). - Return a
&MsgSetDelegationTransferReceiversResponse{}on success.
- Convert the incoming
- Ensure this handler is registered in your module’s
RegisterMsgServercall (usually inmodule.go) so gRPC routing picks it up. - Add unit tests covering valid, empty, and invalid-address cases.
Let me know if you’d like a scaffold for the keeper method and tests.
| func (msg *MsgSetDelegationTransferReceivers) GetSignBytes() []byte { | ||
| bz, _ := json.Marshal(msg) | ||
| return sdk.MustSortJSON(bz) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align GetSignBytes with the standard codec
Other messages use types.ModuleCdc.MustMarshalJSON + sdk.MustSortJSON. Using json.Marshal here is inconsistent and bypasses the module codec.
Apply this diff:
-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, GetSignBytes uses
json.Marshal which bypasses the module codec; replace json.Marshal with the
module codec marshal call: use types.ModuleCdc.MustMarshalJSON(msg) and then
pass the result to sdk.MustSortJSON so signing bytes are produced consistently
with other messages.
| func (msg *MsgSetDelegationTransferReceivers) GetSigners() []sdk.AccAddress { | ||
| sender, _ := sdk.AccAddressFromBech32(msg.Sender) | ||
| return []sdk.AccAddress{sender} | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle address parsing errors in GetSigners to match existing patterns
Other msgs panic on address parsing failures in GetSigners (given ValidateBasic should have run already). Here the error is ignored which could yield an empty address in edge cases.
Apply this diff:
-func (msg *MsgSetDelegationTransferReceivers) GetSigners() []sdk.AccAddress {
- sender, _ := sdk.AccAddressFromBech32(msg.Sender)
- return []sdk.AccAddress{sender}
-}
+func (msg *MsgSetDelegationTransferReceivers) GetSigners() []sdk.AccAddress {
+ sender := sdk.MustAccAddressFromBech32(msg.Sender)
+ 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 := sdk.MustAccAddressFromBech32(msg.Sender) | |
| 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 from AccAddressFromBech32 which can produce
an empty address; change it to handle the parsing error the same way other
messages do by checking the returned error and panicking (or using the project's
established helper) when parsing fails, so the function returns a valid signer
or fails fast if the address is invalid.
Summary by CodeRabbit