[CHORE] sync with master v1.58.2 #329
Conversation
Release/v1 58 for chain v1 16
…function [chore] recover NewTxConfig function
…1.16.69. Added support for MsgSetDelegationTransferReceivers
[CHORE] upgrade protos v1.16.3
WalkthroughAdds new exchange message MsgSetDelegationTransferReceivers across proto and Go, extends StakingKeeper with SetDelegationTransferReceiver, updates client context initialization/TxConfig helpers, adjusts Peggy claim hash composition, bumps Makefile clone tags, updates message registry JSON, adds Params field in exchange v2, and updates proto comments. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MsgSvc as Exchange Msg (v2)
participant Handler
participant StakingKeeper
participant Store
Client->>MsgSvc: MsgSetDelegationTransferReceivers{sender, receivers}
MsgSvc->>Handler: route to exchange module
Handler->>StakingKeeper: SetDelegationTransferReceiver(ctx, receiver) [per receiver]
StakingKeeper->>Store: persist receiver(s)
Handler-->>Client: MsgSetDelegationTransferReceiversResponse
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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: 4
🔭 Outside diff range comments (1)
client/chain/context.go (1)
137-187: Cache the codec/registry with sync.Once to prevent repeated global registrationscreateInjectiveProtoCodec constructs and registers a large set of interfaces. Cache the result to avoid repeated work and ensure consistency across contexts and TxConfig creation.
Suggested change (outside selected lines):
// at file scope var ( codecOnce sync.Once cachedProtoCodec *codec.ProtoCodec cachedInterfaceRegistry types.InterfaceRegistry ) func createInjectiveProtoCodec() (*codec.ProtoCodec, types.InterfaceRegistry) { codecOnce.Do(func() { reg := NewInterfaceRegistry() // ... all RegisterInterfaces(reg) calls ... cachedInterfaceRegistry = reg cachedProtoCodec = codec.NewProtoCodec(reg) }) return cachedProtoCodec, cachedInterfaceRegistry }This keeps the public API unchanged while eliminating redundant allocations and registrations.
🧹 Nitpick comments (9)
Makefile (1)
4-4: Cannot Verify Tags: Remote Repository Not FoundThe
git ls-remotecall failed for:
- injective-indexer @ https://github.com/InjectiveLabs/injective-indexer.git tag v1.16.69
Please manually verify:
- The clone URLs are correct and reachable.
- Tags
v1.16.69(injective-indexer) andv1.16.3(injective-core) exist upstream.- Once confirmed, consider pinning each clone to its exact commit SHA for fully reproducible CI runs.
proto/injective/exchange/v2/exchange.proto (1)
197-199: Additive Params field is backward-compatible; clarify zero-value semantics if relevantAdding human_readable_upgrade_block_height = 32 is wire-safe. If downstream logic treats zero as "unknown" or "not set", consider noting that in the comment to avoid ambiguity in clients.
proto/injective/exchange/v2/market.proto (1)
333-336: Nit: Clarify premium definition and time units in the commentConsider tightening the wording to explicitly state Δt units and clarify what VWAP references (index/oracle vs trade VWAP), e.g., “cumulative_price is ∑ premium_i * Δt_i (Δt_i in seconds), premium_i = (VWAP - mark_price)/mark_price”, to avoid ambiguity for indexers and SDK consumers.
proto/injective/exchange/v2/tx.proto (1)
1524-1534: Add gogoproto options and address scalars for consistency and better tooling supportMost messages here use gogoproto options and, where applicable, cosmos address scalars. Apply the same to this message for parity and better codegen consistency.
Apply this diff:
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; + string sender = 1 [ (cosmos_proto.scalar) = "cosmos.AddressString" ]; // list of receiver addresses to set as delegation transfer receivers - repeated string receivers = 2; + repeated string receivers = 2 [ (cosmos_proto.scalar) = "cosmos.AddressString" ]; }Also ensure admin authorization is enforced in the handler (not proto).
client/chain/context.go (3)
81-85: Avoid constructing a fresh codec/registry in NewTxConfig; accept a marshaler to reuse the one from contextCreating a new registry per call is unnecessary and may lead to inconsistencies/perf overhead. Take a codec.Codec parameter and reuse the caller’s marshaler.
Apply this diff:
-// NewTxConfig initializes new Cosmos TxConfig with certain signModes enabled. -func NewTxConfig(signModes []signingtypes.SignMode) client.TxConfig { - marshaler, _ := createInjectiveProtoCodec() - return tx.NewTxConfig(marshaler, signModes) -} +// NewTxConfig initializes a Cosmos TxConfig with certain signModes enabled using the provided marshaler. +func NewTxConfig(m codec.Codec, signModes []signingtypes.SignMode) client.TxConfig { + return tx.NewTxConfig(m, signModes) +}
95-104: Use the NewTxConfig helper for a single code path and reuse the marshalerUse the helper you introduced to keep TxConfig construction centralized.
Apply this diff:
encodingConfig := EncodingConfig{ InterfaceRegistry: interfaceRegistry, Marshaler: marshaler, - TxConfig: tx.NewTxConfig(marshaler, []signingtypes.SignMode{ - signingtypes.SignMode_SIGN_MODE_DIRECT, - }), + TxConfig: NewTxConfig(marshaler, []signingtypes.SignMode{signingtypes.SignMode_SIGN_MODE_DIRECT}), }
107-126: Minor: trim fromSpec before parsing to avoid surprisesLeading/trailing whitespace in fromSpec can cause bech32 parsing to fail needlessly.
Apply this diff:
+ fromSpec = strings.TrimSpace(fromSpec) addr, err := cosmostypes.AccAddressFromBech32(fromSpec)And add the import:
import ( + "strings"chain/exchange/types/v2/msgs.go (2)
2612-2628: Optionally validate duplicates/self in receiversTo prevent redundant work and guard against accidental self-inclusion, consider rejecting duplicate receiver addresses and the sender address in receivers.
Apply this diff:
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 { + 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 receiver == msg.Sender { + return errors.Wrap(sdkerrors.ErrInvalidRequest, "sender cannot be a delegation transfer receiver") + } + if _, dup := seen[receiver]; dup { + return errors.Wrap(sdkerrors.ErrInvalidRequest, "duplicate receiver address") + } + seen[receiver] = struct{}{} } return nil }
2610-2612: Optional: add a Type constant for the new message to align with the existing patternYou maintain type constants for message Type() values; adding one improves consistency.
Outside selected lines, add:
const ( // ... TypeMsgSetDelegationTransferReceivers = "setDelegationTransferReceivers" )And update:
func (msg *MsgSetDelegationTransferReceivers) Type() string { return TypeMsgSetDelegationTransferReceivers }
📜 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 (10)
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)client/chain/context.go(3 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 (2)
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)
client/chain/context.go (1)
typeddata/typed_data.go (1)
ProtoCodec(813-813)
⏰ 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 (4)
proto/injective/exchange/v1beta1/exchange.proto (1)
408-411: Docs improvement LGTMThe clarified definition for cumulative_price improves consumer understanding. No wire changes.
proto/injective/exchange/v2/tx.proto (2)
222-223: RPC added: ensure server-side Msg handler is implemented and wiredThe RPC addition looks good. Please verify the corresponding Msg server implementation and routing exist in the core chain module, and authorization checks (exchange admin) are enforced there.
1522-1523: LGTM: Response type presenceThe presence of MsgAtomicMarketOrderFeeMultiplierScheduleResponse is consistent with the request message. No issues here.
injective_data/chain_messages_list.json (1)
96-96: LGTM: New message registered in the chain messages listThe new MsgSetDelegationTransferReceivers entry is present, ensuring downstream tooling picks it up.
| SetDelegationTransferReceiver(ctx context.Context, receiver sdk.AccAddress) | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Interface addition is source-breaking for implementers; call out/update mocks
Adding SetDelegationTransferReceiver requires downstream implementers/mocks of StakingKeeper to add this method. Please call this out in release notes and update any local mocks/tests accordingly.
To find potential implementers/mocks that need updates (in this repo):
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Find concrete types that implement or mock StakingKeeper by name.
rg -n "type .*StakingKeeper|var _ .*StakingKeeper|mock.*StakingKeeper" Length of output: 167
Call out interface addition breaking change in release notes
We’ve added SetDelegationTransferReceiver(ctx context.Context, receiver sdk.AccAddress) to the StakingKeeper interface—a source-breaking change for any downstream implementers or mocks. A search revealed no local implementations or mocks of StakingKeeper in this repo, but consumers must:
- Add
SetDelegationTransferReceiverto their ownStakingKeeperimplementations - Update any mocks/tests that assert on this interface
Please document this change in the release notes so integrators can update their code accordingly.
🤖 Prompt for AI Agents
In chain/exchange/types/expected_keepers.go around lines 70-71, the
StakingKeeper interface was extended with SetDelegationTransferReceiver(ctx
context.Context, receiver sdk.AccAddress), which is a breaking API change;
update the release notes to call this out explicitly as a breaking change and
instruct consumers to (1) add SetDelegationTransferReceiver to any downstream
StakingKeeper implementations, (2) update mocks and tests that depend on the
interface to include the new method, and (3) update any integration
documentation or migration guides showing the new method signature and example
usage.
| 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 GetSignBytes to preserve JSON encoding consistency
Other messages use types.ModuleCdc.MustMarshalJSON for signing bytes. Using encoding/json directly can diverge if custom marshalling is introduced.
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, replace the direct
json.Marshal usage in GetSignBytes with the module codec to keep JSON encoding
consistent; call types.ModuleCdc.MustMarshalJSON(msg) to produce the canonical
JSON bytes and then pass those into sdk.MustSortJSON, ensuring imports and the
types.ModuleCdc reference are used instead of encoding/json.
| func (msg *MsgSetDelegationTransferReceivers) GetSigners() []sdk.AccAddress { | ||
| sender, _ := sdk.AccAddressFromBech32(msg.Sender) | ||
| return []sdk.AccAddress{sender} | ||
| } |
There was a problem hiding this comment.
Don’t ignore bech32 parse errors in GetSigners; panic like the rest of the codebase
GetSigners is called prior to ValidateBasic during tx ante-handling. Ignoring the error here may return a nil signer and lead to subtle failures. Match the established pattern and panic on invalid sender.
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, 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
implementation currently ignores the bech32 parse error which can yield a nil
signer; change it to panic on invalid sender consistent with the rest of the
codebase by using the SDK helper that panics on parse failure (or explicitly
check the error and panic), e.g. replace the silent parse with a Must-style
parse or a check that calls panic with the error so an invalid bech32 sender
fails fast.
| // Hash implements WithdrawBatch.Hash | ||
| func (msg *MsgWithdrawClaim) ClaimHash() []byte { | ||
| path := fmt.Sprintf("%s/%d/%d/%s", msg.TokenContract, msg.BatchNonce, msg.EventNonce, msg.TokenContract) | ||
| path := fmt.Sprintf("%s/%d/%d/%d", msg.TokenContract, msg.BatchNonce, msg.EventNonce, msg.BlockHeight) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
ClaimHash composition changed: ensure orchestrators/clients are aligned and document format
Switching the last component from TokenContract to BlockHeight changes the hashed preimage. This is security-sensitive; ensure all off-chain components constructing/validating claims are updated to avoid consensus/verification mismatches. Consider documenting the exact tuple order to prevent regressions.
Apply this diff to make the format explicit in-code:
func (msg *MsgWithdrawClaim) ClaimHash() []byte {
- path := fmt.Sprintf("%s/%d/%d/%d", msg.TokenContract, msg.BatchNonce, msg.EventNonce, msg.BlockHeight)
+ // Format: "<token_contract>/<batch_nonce>/<event_nonce>/<block_height>"
+ path := fmt.Sprintf("%s/%d/%d/%d", msg.TokenContract, msg.BatchNonce, msg.EventNonce, msg.BlockHeight)
return tmhash.Sum([]byte(path))
}To quickly check for other occurrences of hard-coded claim-hash composition in this repo:
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n "ClaimHash\(\)|fmt\.Sprintf\(\"%s/%d/%d/%d|withdraw_claim|deposit_claim" chainLength of output: 952
🏁 Script executed:
#!/usr/bin/env bash
# Display the DepositClaim, WithdrawClaim, and surrounding context
sed -n '360,430p' chain/peggy/types/msgs.goLength of output: 2563
Document ClaimHash tuple formats and audit off-chain alignment
Switching or reordering components in a ClaimHash preimage is security-sensitive: all orchestrators, relayers, and clients must build and verify the exact same string to avoid consensus mismatches. Please:
- Add inline comments for each
ClaimHash()implementation to clearly specify the tuple order. - Audit/update any off-chain code (or client libraries) that construct/validate these claims to match the documented formats.
- Consider centralizing or testing these formats to prevent future regressions.
Affected methods in chain/peggy/types/msgs.go:
- MsgDepositClaim.ClaimHash()
- MsgWithdrawClaim.ClaimHash()
- MsgERC20DeployedClaim.ClaimHash()
- MsgValsetUpdatedClaim.ClaimHash()
Example diff for MsgWithdrawClaim:
func (msg *MsgWithdrawClaim) ClaimHash() []byte {
- path := fmt.Sprintf("%s/%d/%d/%d", msg.TokenContract, msg.BatchNonce, msg.EventNonce, msg.BlockHeight)
+ // Format: "<token_contract>/<batch_nonce>/<event_nonce>/<block_height>"
+ path := fmt.Sprintf("%s/%d/%d/%d", msg.TokenContract, msg.BatchNonce, msg.EventNonce, msg.BlockHeight)
return tmhash.Sum([]byte(path))
}📝 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.
| path := fmt.Sprintf("%s/%d/%d/%d", msg.TokenContract, msg.BatchNonce, msg.EventNonce, msg.BlockHeight) | |
| func (msg *MsgWithdrawClaim) ClaimHash() []byte { | |
| // Format: "<token_contract>/<batch_nonce>/<event_nonce>/<block_height>" | |
| path := fmt.Sprintf("%s/%d/%d/%d", msg.TokenContract, msg.BatchNonce, msg.EventNonce, msg.BlockHeight) | |
| return tmhash.Sum([]byte(path)) | |
| } |
🤖 Prompt for AI Agents
In chain/peggy/types/msgs.go around line 395 (and the other ClaimHash
implementations noted: MsgDepositClaim, MsgWithdrawClaim, MsgERC20DeployedClaim,
MsgValsetUpdatedClaim), the ClaimHash preimage tuple order is not documented;
add an inline comment above each ClaimHash() implementation that enumerates the
exact tuple order and formatting used (e.g., field1/field2/... and any
stringify/zero-padding rules) so orchestrators/relayers/clients can construct
identical strings, update any off‑chain/client code that composes or verifies
these claim preimages to match the documented formats, and centralize the
formatting spec (or helper) and add unit tests that cover the exact string
outputs to prevent future regressions.
devbranch with the latest changes inmasterafter the v1.58.2 releaseSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores