[C-655] add chainlink data streams oracle #342
Conversation
…71. Includes support for the new Chainlink oracle messages.
WalkthroughThis PR adds Chainlink Data Streams support to the oracle, extensive Exchange v2 features (derivative matching, VWAP, fee-discounts, new messages/validators), upgrades protobufs and protos for markets/proposals/permissions, modifies EVM precompile bindings to include Failure events, and refactors EVM log decoding and various APIs. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧬 Code graph analysis (1)examples/chain/permissions/2_MsgUpdateNamespace/example.go (1)
⏰ 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)
🔇 Additional comments (2)
Comment |
There was a problem hiding this comment.
Pull request overview
The pull request adds support for Chainlink Data Streams oracle messages to the Injective chain. This integration extends the existing oracle module to handle a new oracle type alongside existing providers like Pyth and Stork.
Key changes:
- Introduces
ChainlinkDataStreamsoracle type with price state management - Adds RPC methods for relaying Chainlink prices and querying price states
- Implements permissions module enhancements to support separate WASM and EVM contract hooks
- Updates market parameters to support minimal protocol fee disabling
- Adds validation for Chainlink verifier proxy contract addresses
Reviewed changes
Copilot reviewed 61 out of 63 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| proto/injective/oracle/v1beta1/* | Defines Chainlink Data Streams messages, events, and query endpoints |
| proto/injective/permissions/v1beta1/* | Separates contract hooks into wasm_hook and evm_hook fields |
| proto/injective/exchange/v2/* | Adds has_disabled_minimal_protocol_fee field to market proposals |
| chain/oracle/types/params.go | Adds Chainlink verifier proxy contract validation |
| chain/oracle/types/oracle.go | Registers ChainlinkDataStreams oracle type |
| go.mod | Downgrades cometbft dependency version |
| exchange//pb/.go | Generated protobuf code updates |
Comments suppressed due to low confidence (6)
proto/injective/oracle/v1beta1/tx.proto:1
- The comment incorrectly states this message is for updating Pyth prices, but the message is named
MsgRelayChainlinkPrices. Update the comment to reference Chainlink Data Streams instead of Pyth.
proto/injective/oracle/v1beta1/query.proto:1 - Missing space between 'Streams' and 'price' in the comment.
go.mod:1 - The cometbft dependency is being downgraded from v1.0.1-inj.4 to v1.0.1-inj.3. Verify this version downgrade is intentional and won't introduce compatibility issues or missing features needed by this PR.
proto/injective/exchange/v2/proposal.proto:1 - The enum name
DisableMinimalProtocolFeeUpdateis confusing because the valuesFalseandTruesuggest a boolean state rather than an update operation. Consider renaming toMinimalProtocolFeeStatusor using more descriptive value names likeKEEP_CURRENT,ENABLE,DISABLE.
proto/injective/exchange/v2/market.proto:1 - The amino.oneof_name values include the full package prefix. Verify this is the intended format for amino encoding, as the previous version used simpler names like 'uncapped' and 'capped'. This change could affect serialization compatibility.
proto/injective/exchange/v2/market.proto:1 - The deprecation comment provides good context. Ensure migration guides or release notes document this breaking change and guide users to update their code to use the new fields.
💡 Add Copilot custom instructions for smarter, more guided reviews. 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: 7
🧹 Nitpick comments (12)
proto/injective/permissions/v1beta1/params.proto (1)
15-16: Consider naming consistency and gas limit granularity.Two observations:
Naming inconsistency: This field is named
contract_hook_max_gas, whilepermissions.protoandtx.protonow use explicitwasm_hook/evm_hooknaming. Consider whether this should be renamed for consistency (e.g., to a more generic name) or if separate limits are needed.Single gas limit: With distinct WASM and EVM hooks now supported, verify whether a single shared gas limit is intentional. EVM and CosmWasm have different gas semantics, so separate limits (
wasm_hook_max_gas/evm_hook_max_gas) might provide finer control.Makefile (1)
7-7: Verify that the feature branch reference is intentional.The
clone-injective-coretarget now references a feature branch (c-655/add_chainlink_data_streams_oracle) instead of a tagged release. While this is appropriate during development, consider:
- Before merging this PR, the core dependency should likely point to a stable release tag
- Feature branches can be rebased or force-pushed, leading to non-reproducible builds
- If this is intentional for coordinated development across repositories, document the dependency relationship
chain/exchange/types/v2/subaccount.go (1)
77-85: Consider adding nil guard for defensive coding.
AddSubaccountOrderassumesdandd.Orderare non-nil. If called with a nil argument, this will panic. If callers are guaranteed to pass valid data, this is fine; otherwise, consider adding a nil check.🔎 Optional defensive nil check
func (r *SubaccountOrderResults) AddSubaccountOrder(d *SubaccountOrderData) { + if d == nil || d.Order == nil { + return + } if d.Order.IsReduceOnly { r.ReduceOnlyOrders = append(r.ReduceOnlyOrders, d)chain/exchange/types/v2/spot.go (2)
22-35: Silent fallthrough for unknownOrderFillTypemay mask bugs.The default return of
TransientSellOrderbookFillsfor unknown fill types could hide programming errors. Consider returningnilto make unhandled cases explicit, or panic if this represents an invariant violation.🔎 Suggested fix
func (r *SpotOrderbookMatchingResults) GetOrderbookFills(fillType OrderFillType) *OrderbookFills { switch fillType { case RestingLimitBuy: return r.RestingBuyOrderbookFills case RestingLimitSell: return r.RestingSellOrderbookFills case TransientLimitBuy: return r.TransientBuyOrderbookFills case TransientLimitSell: return r.TransientSellOrderbookFills + default: + return nil } - - return r.TransientSellOrderbookFills }
228-240: Minor: simplify variable declaration.The
existingVwapDatadeclaration and immediate assignment can be combined.🔎 Simplified variable initialization
func (p *SpotVwapInfo) ApplyVwap(marketID common.Hash, newVwapData *SpotVwapData) { - var existingVwapData *SpotVwapData - - existingVwapData = (*p)[marketID] + existingVwapData := (*p)[marketID] if existingVwapData == nil { existingVwapData = NewSpotVwapData() (*p)[marketID] = existingVwapData }chain/exchange/types/v2/fee_discounts.go (1)
274-321: Incorrect slice capacity and inefficient sort comparison.
Line 278 uses
len(info.AccountFeeTiers)as capacity forsubaccountVolumes, but iterates over the nestedSubaccountMarketVolumeContributionsmap.Lines 301-305 use
appendinside the sort comparison function, which creates temporary allocations on every comparison call. Consider pre-computing the concatenated keys.🔎 Proposed optimization
func (info *FeeDiscountStakingInfo) GetSortedSubaccountAndMarketVolumes() ( []*SubaccountVolumeContribution, []*MarketVolumeContribution, ) { - subaccountVolumes := make([]*SubaccountVolumeContribution, 0, len(info.AccountFeeTiers)) + subaccountVolumes := make([]*SubaccountVolumeContribution, 0) marketVolumeTracker := make(map[common.Hash]VolumeRecord) // ... iteration code unchanged ... info.AccountVolumesMux.RUnlock() + // Pre-compute sort keys to avoid allocations in comparison + type sortKey struct { + idx int + key []byte + } + keys := make([]sortKey, len(subaccountVolumes)) + for i, sv := range subaccountVolumes { + keys[i] = sortKey{idx: i, key: append(sv.SubaccountID.Bytes(), sv.MarketID.Bytes()...)} + } + sort.SliceStable(keys, func(i, j int) bool { + return bytes.Compare(keys[i].key, keys[j].key) < 0 + }) + sorted := make([]*SubaccountVolumeContribution, len(subaccountVolumes)) + for i, k := range keys { + sorted[i] = subaccountVolumes[k.idx] + } + subaccountVolumes = sorted - sort.SliceStable(subaccountVolumes, func(i, j int) bool { - return bytes.Compare( - append(subaccountVolumes[i].SubaccountID.Bytes(), subaccountVolumes[i].MarketID.Bytes()...), - append(subaccountVolumes[j].SubaccountID.Bytes(), subaccountVolumes[j].MarketID.Bytes()...), - ) < 0 - })proto/injective/exchange/v2/proposal.proto (1)
82-86: Consider clarifying field naming for readability.The field
has_disabled_minimal_protocol_feeuses a double-negative construction ("disabled" + negative enum values). While functionally correct and likely matching the market type's field name, it can be confusing:
Truemeans "fee is disabled" (no fee)Falsemeans "fee is not disabled" (fee applies)If this naming is intentional to match the market state field, consider adding a brief comment to clarify the semantics. Otherwise, a positive framing like
minimal_protocol_fee_enabledwith inverted enum values might be clearer.Also applies to: 490-493, 590-593
proto/injective/oracle/v1beta1/tx.proto (1)
167-181: Fix stale comment aboveMsgRelayChainlinkPricesThe comment still references Pyth instead of Chainlink, which can confuse readers.
Proposed comment fix
-// MsgRelayPythPrices defines a SDK message for updating Pyth prices +// MsgRelayChainlinkPrices defines a SDK message for relaying Chainlink prices message MsgRelayChainlinkPrices {chain/oracle/types/msgs.go (1)
406-433: MsgRelayChainlinkPrices wiring mirrors existing Pyth relay semantics
Route,Type,GetSignBytes, andGetSignersare implemented consistently with the other oracle messages, andValidateBasiccorrectly enforces a valid bech32 sender address.If the handler relies on at least one
reportsentry, you may optionally enforcelen(msg.Reports) > 0here to fail fast on obviously empty relays; otherwise, current behaviour matches the Pyth relay pattern.chain/exchange/types/v2/msgs.go (1)
965-973: Centralized validation for market orders; post‑only now rejected for market messagesThe new helpers and their wiring introduce a clear, consistent validation policy:
ValidateSpotMarketOrder/ValidateDerivativeMarketOrder/ValidateBinaryOptionsMarketOrder:
- Reject
BUY_PO/SELL_POfor all market orders, which aligns with “post‑only” being a limit‑order concept.- For binary options, additionally reject conditional types for market orders and reuse the existing binary band validation via
order.ValidateBasic(senderAddr, true).MsgCreateSpotMarketOrder,MsgCreateDerivativeMarketOrder, andMsgCreateBinaryOptionsMarketOrdernow delegate to these helpers.MsgBatchUpdateOrders.ValidateBasicnow applies the same helpers toSpotMarketOrdersToCreate,DerivativeMarketOrdersToCreate, andBinaryOptionsMarketOrdersToCreate, ensuring single and batched flows share identical rules.Net effect: any client that previously sent post‑only market orders will now receive
ErrInvalidOrderTypeForMessage. That looks intentional but is a behavioural change worth being aware of.As a minor defensive improvement, you could also nil‑check the
orderpointer in each helper before dereferencing, in case a malformed batch ever contained a nil entry.Also applies to: 2110-2126, 2727-2752
chain/exchange/types/v2/derivative.go (2)
906-908: Remove commented-out debug code.This commented-out debug statement should be removed before merging.
🔎 Proposed fix
if position.Position != nil { positions = append(positions, position.Position) nonNilPositionSubaccountIDs = append(nonNilPositionSubaccountIDs, subaccountID) } - - // else { - // fmt.Println("❌ position is nil for subaccount", subaccountID.Hex()) - // } }
134-146: Consider documenting the nil position indicator pattern.
SetPositionsilently returns whenpositionis nil, whileSetPositionIndicatorexplicitly sets a nil position. This asymmetry could lead to confusion. Consider adding a brief comment explaining the intended usage pattern for position indicators.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (20)
chain/exchange/types/exchange.pb.gois excluded by!**/*.pb.gochain/exchange/types/query.pb.gois excluded by!**/*.pb.gochain/exchange/types/v2/market.pb.gois excluded by!**/*.pb.gochain/exchange/types/v2/proposal.pb.gois excluded by!**/*.pb.gochain/exchange/types/v2/query.pb.gois excluded by!**/*.pb.gochain/oracle/types/events.pb.gois excluded by!**/*.pb.gochain/oracle/types/genesis.pb.gois excluded by!**/*.pb.gochain/oracle/types/oracle.pb.gois excluded by!**/*.pb.gochain/oracle/types/query.pb.gois excluded by!**/*.pb.gochain/oracle/types/tx.pb.gois excluded by!**/*.pb.gochain/permissions/types/params.pb.gois excluded by!**/*.pb.gochain/permissions/types/permissions.pb.gois excluded by!**/*.pb.gochain/permissions/types/tx.pb.gois excluded by!**/*.pb.goexchange/auction_rpc/pb/goadesign_goagen_injective_auction_rpc.pb.gois excluded by!**/*.pb.goexchange/auction_rpc/pb/goadesign_goagen_injective_auction_rpc_grpc.pb.gois excluded by!**/*.pb.goexchange/derivative_exchange_rpc/pb/goadesign_goagen_injective_derivative_exchange_rpc.pb.gois excluded by!**/*.pb.goexchange/event_provider_api/pb/goadesign_goagen_event_provider_api.pb.gois excluded by!**/*.pb.goexchange/oracle_rpc/pb/goadesign_goagen_injective_oracle_rpc.pb.gois excluded by!**/*.pb.goexchange/portfolio_rpc/pb/goadesign_goagen_injective_portfolio_rpc.pb.gois excluded by!**/*.pb.gogo.sumis excluded by!**/*.sum
📒 Files selected for processing (43)
Makefilechain/evm/precompiles/bank/fixed_supply_bank_erc20.abigen.gochain/evm/precompiles/bank/i_bank_module.abigen.gochain/evm/precompiles/bank/mint_burn_bank_erc20.abigen.gochain/evm/precompiles/exchange/i_exchange_module.abigen.gochain/evm/precompiles/staking/i_staking_module.abigen.gochain/evm/types/events.gochain/evm/types/utils.gochain/exchange/types/codec.gochain/exchange/types/errors.gochain/exchange/types/expected_keepers.gochain/exchange/types/msgs.gochain/exchange/types/proposal.gochain/exchange/types/v2/authz_exchange_generic.gochain/exchange/types/v2/codec.gochain/exchange/types/v2/derivative.gochain/exchange/types/v2/fee_discounts.gochain/exchange/types/v2/market.gochain/exchange/types/v2/msgs.gochain/exchange/types/v2/proposal.gochain/exchange/types/v2/spot.gochain/exchange/types/v2/subaccount.gochain/exchange/types/v2/trade.gochain/oracle/types/errors.gochain/oracle/types/msgs.gochain/oracle/types/oracle.gochain/oracle/types/params.gogo.modinjective_data/chain_messages_list.jsoninjective_data/ofac.jsonproto/injective/exchange/v1beta1/exchange.protoproto/injective/exchange/v1beta1/query.protoproto/injective/exchange/v2/market.protoproto/injective/exchange/v2/proposal.protoproto/injective/exchange/v2/query.protoproto/injective/oracle/v1beta1/events.protoproto/injective/oracle/v1beta1/genesis.protoproto/injective/oracle/v1beta1/oracle.protoproto/injective/oracle/v1beta1/query.protoproto/injective/oracle/v1beta1/tx.protoproto/injective/permissions/v1beta1/params.protoproto/injective/permissions/v1beta1/permissions.protoproto/injective/permissions/v1beta1/tx.proto
🧰 Additional context used
🧬 Code graph analysis (16)
chain/oracle/types/errors.go (1)
chain/oracle/types/key.go (1)
ModuleName(12-12)
chain/exchange/types/msgs.go (1)
chain/exchange/types/errors.go (2)
ErrInvalidAddress(91-91)ErrInvalidStakeGrant(110-110)
chain/exchange/types/v2/trade.go (1)
chain/exchange/types/trading_rewards.go (1)
TradingRewardPoints(9-9)
chain/exchange/types/errors.go (6)
chain/exchange/types/key.go (1)
ModuleName(17-17)chain/peggy/types/key.go (1)
ModuleName(10-10)chain/permissions/types/codec.go (1)
ModuleName(12-12)chain/oracle/types/key.go (1)
ModuleName(12-12)chain/wasmx/types/key.go (1)
ModuleName(10-10)chain/ocr/types/key.go (1)
ModuleName(12-12)
chain/exchange/types/codec.go (4)
chain/exchange/types/tx.pb.go (15)
MsgDecreasePositionMargin(2899-2911)MsgDecreasePositionMargin(2915-2915)MsgDecreasePositionMargin(2916-2918)MsgLiquidatePosition(2573-2582)MsgLiquidatePosition(2586-2586)MsgLiquidatePosition(2587-2589)MsgEmergencySettleMarket(2683-2690)MsgEmergencySettleMarket(2694-2694)MsgEmergencySettleMarket(2695-2697)MsgAuthorizeStakeGrants(3504-3507)MsgAuthorizeStakeGrants(3511-3511)MsgAuthorizeStakeGrants(3512-3514)MsgActivateStakeGrant(3593-3596)MsgActivateStakeGrant(3600-3600)MsgActivateStakeGrant(3601-3603)chain/exchange/types/v2/tx.pb.go (15)
MsgDecreasePositionMargin(3050-3062)MsgDecreasePositionMargin(3066-3066)MsgDecreasePositionMargin(3067-3069)MsgLiquidatePosition(2617-2626)MsgLiquidatePosition(2630-2630)MsgLiquidatePosition(2631-2633)MsgEmergencySettleMarket(2834-2841)MsgEmergencySettleMarket(2845-2845)MsgEmergencySettleMarket(2846-2848)MsgAuthorizeStakeGrants(3658-3663)MsgAuthorizeStakeGrants(3667-3667)MsgAuthorizeStakeGrants(3668-3670)MsgActivateStakeGrant(3749-3754)MsgActivateStakeGrant(3758-3758)MsgActivateStakeGrant(3759-3761)chain/exchange/types/v2/market.pb.go (4)
OpenNotionalCap_Uncapped(109-111)OpenNotionalCap_Uncapped(116-116)OpenNotionalCap_Capped(112-114)OpenNotionalCap_Capped(117-117)chain/exchange/types/v2/query.pb.go (4)
FullDerivativeMarket_PerpetualInfo(3321-3323)FullDerivativeMarket_PerpetualInfo(3328-3328)FullDerivativeMarket_FuturesInfo(3324-3326)FullDerivativeMarket_FuturesInfo(3329-3329)
chain/exchange/types/expected_keepers.go (2)
chain/oracle/types/oracle.pb.go (5)
OracleType(32-32)OracleType(89-91)PriceRecords(1304-1308)PriceRecords(1312-1312)PriceRecords(1313-1315)chain/oracle/types/query.pb.go (6)
ScalingOptions(1604-1607)ScalingOptions(1611-1611)ScalingOptions(1612-1614)PricePairState(1726-1734)PricePairState(1738-1738)PricePairState(1739-1741)
chain/exchange/types/v2/msgs.go (1)
chain/exchange/types/errors.go (4)
ErrInvalidAddress(91-91)ErrInvalidStakeGrant(110-110)ErrInvalidOrderTypeForMessage(68-68)ErrUnrecognizedOrderType(15-15)
chain/exchange/types/v2/subaccount.go (1)
chain/exchange/types/exchange.pb.go (3)
SubaccountOrderData(1812-1815)SubaccountOrderData(1819-1819)SubaccountOrderData(1820-1822)
chain/exchange/types/v2/codec.go (4)
chain/exchange/types/v2/tx.pb.go (12)
MsgDecreasePositionMargin(3050-3062)MsgDecreasePositionMargin(3066-3066)MsgDecreasePositionMargin(3067-3069)MsgEmergencySettleMarket(2834-2841)MsgEmergencySettleMarket(2845-2845)MsgEmergencySettleMarket(2846-2848)MsgAuthorizeStakeGrants(3658-3663)MsgAuthorizeStakeGrants(3667-3667)MsgAuthorizeStakeGrants(3668-3670)MsgActivateStakeGrant(3749-3754)MsgActivateStakeGrant(3758-3758)MsgActivateStakeGrant(3759-3761)chain/exchange/types/v2/proposal.pb.go (3)
DenomMinNotionalProposal(1260-1264)DenomMinNotionalProposal(1268-1268)DenomMinNotionalProposal(1269-1271)chain/exchange/types/v2/market.pb.go (4)
OpenNotionalCap_Uncapped(109-111)OpenNotionalCap_Uncapped(116-116)OpenNotionalCap_Capped(112-114)OpenNotionalCap_Capped(117-117)chain/exchange/types/v2/query.pb.go (4)
FullDerivativeMarket_PerpetualInfo(3321-3323)FullDerivativeMarket_PerpetualInfo(3328-3328)FullDerivativeMarket_FuturesInfo(3324-3326)FullDerivativeMarket_FuturesInfo(3329-3329)
chain/oracle/types/oracle.go (1)
chain/oracle/types/oracle.pb.go (1)
OracleType_ChainlinkDataStreams(48-48)
chain/exchange/types/proposal.go (5)
chain/exchange/types/v2/proposal.pb.go (3)
AdminInfo(614-617)AdminInfo(621-621)AdminInfo(622-624)chain/exchange/types/proposal.pb.go (3)
AdminInfo(566-569)AdminInfo(573-573)AdminInfo(574-576)chain/oracle/types/oracle.pb.go (2)
OracleType_Stork(47-47)OracleType_ChainlinkDataStreams(48-48)chain/exchange/types/errors.go (2)
ErrInvalidAddress(91-91)ErrInvalidPermissions(112-112)chain/exchange/types/market_admin.go (1)
MaxPerm(13-14)
chain/oracle/types/params.go (1)
chain/oracle/types/oracle.pb.go (3)
Params(93-98)Params(102-102)Params(103-105)
chain/evm/precompiles/bank/mint_burn_bank_erc20.abigen.go (1)
ethereum/util/contract.go (1)
BoundContract(27-35)
chain/exchange/types/v2/proposal.go (4)
chain/exchange/types/v2/proposal.pb.go (6)
DisableMinimalProtocolFeeUpdate(33-33)DisableMinimalProtocolFeeUpdate(57-59)DisableMinimalProtocolFeeUpdate_NoUpdate(36-36)AdminInfo(614-617)AdminInfo(621-621)AdminInfo(622-624)chain/oracle/types/oracle.pb.go (4)
OracleType_BandIBC(45-45)OracleType_Provider(46-46)OracleType_Stork(47-47)OracleType_ChainlinkDataStreams(48-48)chain/exchange/types/errors.go (2)
ErrInvalidAddress(91-91)ErrInvalidPermissions(112-112)chain/exchange/types/market_admin.go (1)
MaxPerm(13-14)
chain/exchange/types/v2/derivative.go (6)
chain/exchange/types/v2/market.go (2)
DerivativeMarket(211-213)DerivativeMarket(223-225)chain/exchange/types/deposit.go (3)
NewDepositDeltas(42-44)DepositDeltas(40-40)DepositDelta(11-14)chain/exchange/types/trading_rewards.go (2)
NewTradingRewardPoints(11-15)TradingRewardPoints(9-9)chain/exchange/types/market.go (1)
NotionalToChainFormat(130-136)chain/exchange/types/v2/common_utils.go (2)
DerivativeLimitOrderDelta(15-19)DerivativeMarketOrderDelta(21-24)exchange/portfolio_rpc/pb/goadesign_goagen_injective_portfolio_rpc.pb.go (3)
DerivativePosition(635-672)DerivativePosition(687-687)DerivativePosition(702-704)
chain/evm/precompiles/bank/fixed_supply_bank_erc20.abigen.go (1)
ethereum/util/contract.go (1)
BoundContract(27-35)
⏰ 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). (3)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: run-tests
- GitHub Check: lint
🔇 Additional comments (69)
proto/injective/permissions/v1beta1/permissions.proto (1)
13-14: LGTM – EVM hook support added correctly.The field renaming from
contract_hooktowasm_hook(field 2) and addition ofevm_hook(field 8) are well-structured. Field numbers are preserved, ensuring protobuf wire compatibility.Note: Renaming
contract_hook→wasm_hookchanges the JSON field name, which could be a breaking change for clients using JSON encoding. Ensure downstream consumers are updated accordingly.Also applies to: 26-28
proto/injective/permissions/v1beta1/tx.proto (1)
65-77: LGTM – Consistent with permissions.proto changes.The
SetContractHooktype is appropriately reused for bothwasm_hook(field 3) andevm_hook(field 8). Field numbers are preserved, maintaining wire compatibility.Same consideration as noted in
permissions.proto: the JSON field name change fromcontract_hooktowasm_hookmay affect JSON-based clients.go.mod (1)
256-256: No action required. The cometbft version change is part of the coordinated C-655 PR that updates proto definitions to chain v1.18.0 and indexer v1.17.71 and adds Chainlink oracle support, as evidenced by the oracle.proto file changes showing new ChainlinkDataStreams enum and Chainlink-related configuration parameters.chain/oracle/types/errors.go (1)
51-51: LGTM!The new error definition follows the existing error registration pattern and provides a clear message for Chainlink report verification failures. The error code (44) is sequential and appropriate.
chain/evm/precompiles/staking/i_staking_module.abigen.go (1)
40-40: LGTM - Generated code with improved ABI format.The ABI string has been reformatted to use explicit field names (
type,name,internalType), which improves readability while maintaining functional equivalence. Since this is generated code (as indicated by the file header), the change results from regenerating the bindings with an updated toolchain.chain/evm/precompiles/bank/i_bank_module.abigen.go (1)
34-34: LGTM - Generated code with improved ABI format.Consistent with the staking module binding, the ABI string has been reformatted to use explicit field names. This is generated code with improved readability and no functional changes.
chain/exchange/types/errors.go (1)
120-121: LGTM!The two new error definitions follow the established error registration pattern and use sequential error codes (112, 113). The error messages clearly describe the failure conditions related to open notional cap validation and offsetting position requirements.
chain/evm/types/events.go (1)
15-15: No internal references to removed constants found.The constants
EventTypeTxLogandAttributeKeyTxLogare not referenced elsewhere in the codebase, so the removal does not break internal dependencies. If these constants are part of a public API, ensure external consumers are notified of the deprecation or provide migration documentation separately.proto/injective/exchange/v1beta1/exchange.proto (1)
23-28: Update amino.oneof_name changes to use fully qualified names for JSON encoding consistency.The
amino.oneof_nameattributes have been updated to fully qualified names (injective.exchange.v1beta1.OpenNotionalCapUncappedandinjective.exchange.v1beta1.OpenNotionalCapCapped). This improves naming consistency in amino JSON output but does not affect protobuf wire format compatibility (field numbers remain unchanged). No migration strategy is required unless downstream systems explicitly depend on parsing the old short-form strings from amino JSON output.proto/injective/exchange/v2/query.proto (1)
265-265: The HTTP endpoint paths in the v2 proto file show version prefixes consistent with the v2 API structure. However, PositionsInMarket is a new endpoint introduced in V2 (not present in v1beta1), and SubaccountPositions maintained its endpoint structure with only the version prefix updated from v1beta1 to v2. These are not breaking changes to existing REST API endpoints but rather additions and version-specific routing, which is the expected behavior for versioned APIs.Likely an incorrect or invalid review comment.
chain/exchange/types/proposal.go (3)
1655-1665: LGTM! Clean validation implementation for AdminInfo.The
ValidateBasicmethod correctly:
- Validates the Admin address using
sdk.AccAddressFromBech32only when non-empty- Validates
AdminPermissionsagainstMaxPermto ensure no invalid permission bits are set- Uses appropriate error types from the errors registry (
ErrInvalidAddress,ErrInvalidPermissions)
679-686: LGTM! New oracle type correctly added to validation.The
OracleType_ChainlinkDataStreamsis properly included in the valid oracle types switch case, consistent with its definition in the oracle protobuf (OracleType_ChainlinkDataStreams = 13).
290-294: LGTM! Consistent AdminInfo validation integration across proposals.The
AdminInfo.ValidateBasic()call is consistently integrated into all relevant proposal types with proper nil-check and early error return. This centralizes admin validation logic and maintains consistency.Also applies to: 428-432, 535-539, 802-807, 903-908
chain/oracle/types/params.go (3)
55-59: Verify the defaultAcceptUnverifiedChainlinkDataStreamsReports = true.Setting this to
trueby default means unverified Chainlink Data Streams reports will be accepted when no verifier proxy contract is configured. This may be intentional for initial rollout or testing, but please confirm this is the desired security posture for production.
116-132: LGTM! Proper Ethereum address validation.The
ValidateChainlinkVerifierProxyContractfunction correctly:
- Accepts
interface{}for compatibility with param validation patterns- Allows empty string for optional configuration
- Uses
common.IsHexAddressfrom go-ethereum for robust hex address validation
72-81: LGTM! Validation properly extended for new params.The
Validate()method correctly validates both contract addresses with descriptive error wrapping to identify which field failed.chain/evm/types/utils.go (2)
87-97: LGTM! Simplified API with proper bounds checking.The
DecodeMsgLogsfunction now has a cleaner signature and includes proper bounds validation formsgIndex(lines 93-95) to prevent out-of-range access. The function correctly returns an error for invalid indices rather than panicking.
99-110: LGTM! Clean aggregation of logs across all responses.The
DecodeTxLogsfunction correctly iterates through all transaction responses and aggregates logs using thelogsFromTxResponsehelper, maintaining consistency with the single-message variant.proto/injective/oracle/v1beta1/oracle.proto (3)
157-163: Verify intentional type difference forfeed_idbetween messages.
ChainlinkDataStreamsPriceState.feed_idis defined asstring(line 158), whileChainlinkReport.feed_idis defined asbytes(line 314).This may be intentional (e.g.,
ChainlinkReportholds raw data from the report whileChainlinkDataStreamsPriceStatestores a hex-encoded or readable version), but please confirm this is the intended design.Also applies to: 313-318
11-19: LGTM! Params extended with proper field numbering.The new Chainlink-related fields are added with sequential field numbers (2, 3, 4), maintaining protobuf wire compatibility. The field types are appropriate:
stringfor the contract addressboolfor the feature flaguint64for the gas limit
21-36: LGTM! New oracle type properly added to enum.
ChainlinkDataStreams = 13is correctly added following the existing sequence and maintains backward compatibility with existing enum values.chain/exchange/types/expected_keepers.go (1)
31-59: Breaking interface changes require all external implementations to be updated.The
OracleKeeperinterface in this SDK has breaking changes that affect any code implementing these methods:
GetPricePairStatenow requiresscalingOptions *oracletypes.ScalingOptionsparameterGetCumulativePricereturns two values:(baseCumulative, quoteCumulative *sdkmath.LegacyDec)GetHistoricalPriceRecordsreturns(entry *oracletypes.PriceRecords, omitted bool)GetMixedHistoricalPriceRecordsreturns(mixed *oracletypes.PriceRecords, ok bool)Any consumer of this SDK that implements the OracleKeeper interface must update their implementations to match these signatures.
chain/oracle/types/oracle.go (1)
40-41: LGTM!The new
chainlinkdatastreamscase follows the established pattern of other oracle types in the switch statement.proto/injective/oracle/v1beta1/genesis.proto (1)
45-47: LGTM!The new
chainlink_data_streams_price_statesfield follows the established pattern and uses the next sequential field number.chain/exchange/types/v2/subaccount.go (1)
47-67: LGTM!The constructor functions properly initialize all
LegacyDecfields to zero and pre-allocate slices, following good initialization patterns.proto/injective/exchange/v2/market.proto (3)
335-344: Well-documented deprecation.The deprecation includes clear rationale and points to the new replacement fields. Retaining the field number ensures backward compatibility with existing data.
351-364: LGTM!The new
expiration_twap_start_base_cumulative_priceandexpiration_twap_start_quote_cumulative_pricefields properly address the limitation of the deprecated field by tracking base and quote cumulative prices separately.
109-111: Consistent addition ofhas_disabled_minimal_protocol_feeacross market types.The field is added uniformly to
SpotMarket,BinaryOptionsMarket, andDerivativeMarketwith consistent naming and documentation.Also applies to: 187-190, 278-281
chain/exchange/types/v2/spot.go (3)
204-219: LGTM on VWAP calculation logic.The
ApplyExecutionmethod correctly handles nil receivers by creating a new instance, and guards against nil/zero quantity inputs. The weighted average calculation(oldPrice * oldQty + newPrice * newQty) / totalQtyis mathematically correct for VWAP.
93-124: LGTM on deposit delta handling.The
UpdateFromDepositDeltasmethod correctly handles:
- Positive change amounts being added to available balance
- Zero subaccount fallback to auction subaccount for fee recipients
- Proper application of trader and fee recipient deltas
126-190: LGTM on batch execution event construction.The function properly:
- Preallocates slices with appropriate capacity
- Skips zero-fill trades to avoid empty entries
- Handles self-relayed trade fee calculation correctly
- Returns nil event when no trades occurred
chain/exchange/types/v2/fee_discounts.go (5)
84-95: LGTM - ValidatorCache type and NewFeeDiscountConfig constructor.The defensive nil check for
stakingInfoand settingisQualified = falseis appropriate.
97-121: LGTM - FeeDiscountConfig struct and GetFeeDiscountRate method.Proper nil receiver check and correct
RLock/RUnlockpattern for concurrent access.
123-162: LGTM - IncrementAccountVolumeContribution implementation.Defensive check for negative amounts, proper mutex locking, and correct map update patterns.
164-217: LGTM - FeeDiscountStakingInfo struct and constructor.All maps and mutexes are properly initialized.
323-383: LGTM - Remaining getter/setter methods.Proper mutex usage throughout.
SetAccountTierInfo,SetNewAccountTierTTL,AddCheckpoint, andAddInvalidGrantall correctly use Lock/Unlock.injective_data/chain_messages_list.json (1)
128-128: LGTM - New Chainlink message type registration.The new
MsgRelayChainlinkPricesmessage type is correctly added and properly positioned alphabetically within the oracle module namespace. This aligns with the PR objective of adding Chainlink data streams oracle support.proto/injective/oracle/v1beta1/events.proto (1)
90-93: LGTM - New Chainlink data streams event.The
EventSetChainlinkDataStreamsPricesmessage follows the established pattern used by similar events (EventSetStorkPrices,EventSetPythPrices) and correctly references theChainlinkDataStreamsPriceStatetype from the importedoracle.proto.chain/exchange/types/msgs.go (1)
2099-2111: LGTM - Duplicate grantee validation.The implementation correctly detects and rejects duplicate grantees within a single
MsgAuthorizeStakeGrantsmessage. The error message includes the duplicate grantee address for debugging, and the validation is performed before the amount validation.chain/exchange/types/v2/trade.go (1)
1-23: LGTM - TradeFeeData struct and constructor.Clean implementation with proper initialization using
math.LegacyZeroDec(). The struct provides a clear model for trade fee-related data, and the constructor correctly accepts the discounted rate while initializing other fields to zero.chain/evm/precompiles/exchange/i_exchange_module.abigen.go (1)
1-2: Generated file - verify ABI source consistency.This is an auto-generated binding file. The ABI update on line 206 (changing
uint256toExchangeTypes.UFixed256x18for price, quantity, margin, and related fields) appears consistent with the PR's goal of updating proto definitions for chain v1.18.0.Ensure the Solidity interface source (
IExchangeModule.solor equivalent) was updated first, and this file was regenerated usingabigenrather than manually edited.Also applies to: 206-206
chain/exchange/types/v2/authz_exchange_generic.go (2)
115-118: Verify intended behavior when hold is missing from context.When
getHoldreturnsok = false(context key not set or value is nil), the method returnsAccept: false. This means authorization will be denied if the caller forgets to set the hold in context.Is this the intended behavior? Consider whether:
- Missing hold should be treated as zero spend (accept with unchanged limit), or
- Missing hold should explicitly reject (current behavior)
If the current strict behavior is intentional, consider adding a comment explaining why missing hold results in rejection.
110-131: LGTM - Clean spend limit enforcement logic.The refactored
Acceptmethod implements a clear flow:
- No-limit case returns early (line 111-113)
- Missing hold context rejects authorization (line 115-118)
- Single-pass validation and deduction of spend limits (line 120-130)
The logic correctly handles the case where a hold denom isn't in the spend limit (returns false since
AmountOfreturns zero for missing denoms).proto/injective/oracle/v1beta1/query.proto (1)
226-234: LGTM - New Chainlink query messages.The new request/response message definitions follow the established patterns in this file (empty request for retrieving all states, response with repeated price state field).
chain/exchange/types/codec.go (3)
18-20: LGTM - Oneof interface registration for Amino.Registering oneof interfaces (
isOpenNotionalCap_CapandisFullDerivativeMarket_Info) is necessary for proper Amino JSON serialization of protobuf oneof fields. This follows the correct pattern for handling oneofs in the Cosmos SDK legacy Amino codec.
41-45: LGTM - New message type registrations.The new messages (
MsgDecreasePositionMargin,MsgEmergencySettleMarket,MsgAuthorizeStakeGrants,MsgActivateStakeGrant) are correctly registered in both:
- Legacy Amino codec (lines 41-45) for JSON serialization
- Interface registry (lines 122-126) for protobuf/gRPC
Registration names follow the established
exchange/MsgXconvention.Also applies to: 122-126
92-98: LGTM - Oneof implementation registrations.Concrete oneof types are registered with clear, descriptive names that distinguish between the variants:
exchange/OpenNotionalCapUncappedandexchange/OpenNotionalCapCappedexchange/FullDerivativeMarketPerpetualInfoandexchange/FullDerivativeMarketFuturesInfoThis enables proper Amino serialization/deserialization of these polymorphic types.
proto/injective/exchange/v2/proposal.proto (1)
17-21: LGTM - Tri-state enum for optional updates.The
DisableMinimalProtocolFeeUpdateenum correctly implements a tri-state pattern:
NoUpdate (0): Don't modify the existing valueFalse (1): Set the flag to false (enable minimal protocol fee)True (2): Set the flag to true (disable minimal protocol fee)This is the appropriate pattern for optional fields in update proposals where you need to distinguish between "don't change" and "set to false".
chain/evm/precompiles/bank/fixed_supply_bank_erc20.abigen.go (2)
34-35: Generated ABI/Bin update – no manual review neededThese large ABI/Bin string literals are generated and appear consistent with the added Failure event bindings; no manual changes suggested.
608-741: Failure event bindings mirror existing iterator/watch patterns correctlyThe
FixedSupplyBankERC20FailureIterator,FixedSupplyBankERC20Failurestruct, and theFilterFailure/WatchFailure/ParseFailuremethods follow the same patterns as the existing Approval/Transfer bindings and correctly useFilterLogs/WatchLogswithout indexed topics. Looks good.proto/injective/oracle/v1beta1/tx.proto (1)
46-50: Chainlink RPC wiring matches existing oracle Msg patterns
RelayChainlinkPricesis added consistently with other RPCs (Pyth, Stork, etc.) and uses the expected request/response message types. No issues here.chain/oracle/types/msgs.go (1)
27-27: Chainlink relay type and sdk.Msg registration are consistent
TypeMsgRelayChainlinkPricesand the_ sdk.Msg = &MsgRelayChainlinkPrices{}registration align with the existing oracle message conventions.Also applies to: 39-39
chain/evm/precompiles/bank/mint_burn_bank_erc20.abigen.go (2)
34-35: Generated ABI/Bin update – treated as codegenThe updated ABI/Bin literals reflect the new
Failureevent; assuming these came from regenerating bindings, there’s nothing to adjust manually.
744-877: MintBurn Failure event bindings are correct and consistentThe
MintBurnBankERC20FailureIterator,MintBurnBankERC20Failurestruct, and associatedFilterFailure/WatchFailure/ParseFailuremethods follow the established iterator and subscription patterns and correctly omit indexed topics for this event. All good.chain/exchange/types/v2/msgs.go (2)
346-360: Stork timestamp comment accurately documents existing behaviourThe new comment about converting timestamps to nanoseconds matches the surrounding code and improves readability without changing semantics.
2227-2243: Duplicate grantee detection in MsgAuthorizeStakeGrantsThe new
seenGranteesmap cleanly prevents duplicateGranteeentries within a singleMsgAuthorizeStakeGrants, using validated bech32 strings as keys and returningErrInvalidStakeGranton duplicates. This is a sensible tightening of validation with minimal overhead.chain/exchange/types/v2/codec.go (2)
38-40: LGTM! Oneof interface and concrete type registrations are properly structured.The registration of oneof interfaces (
isOpenNotionalCap_Cap,isFullDerivativeMarket_Info) and their concrete implementations (OpenNotionalCap_Uncapped,OpenNotionalCap_Capped,FullDerivativeMarket_PerpetualInfo,FullDerivativeMarket_FuturesInfo) follows the correct pattern for Amino codec compatibility with protobuf oneof fields.Also applies to: 120-127
61-77: LGTM! New message types are properly registered.The new messages (
MsgDecreasePositionMargin,MsgEmergencySettleMarket,MsgAuthorizeStakeGrants,MsgActivateStakeGrant) are correctly registered in bothRegisterLegacyAminoCodecandRegisterInterfaces, ensuring proper serialization and interface compliance.Also applies to: 151-167
chain/exchange/types/v2/market.go (3)
107-110: LGTM! Maturation start condition correctly updated.The
IsStartingMaturationmethod now properly checksExpirationTwapStartBaseCumulativePriceto determine if maturation is starting, which aligns with the TWAP-based calculation approach for expiry futures markets.
371-394: LGTM! Market solvency and balance delta utilities are well-implemented.
IsMarketSolventprovides a clear check for market fund adequacy.GetMarketBalanceDeltacorrectly handles the reduce-only case by adding the trade fee back before computing the delta, as reduce-only orders have fees deducted from payout rather than held separately.
12-16: LGTM! NewMarketIDQuoteDenomMakerFeestruct is well-defined.The struct provides a clean grouping of market identification and maker fee information for fee-related operations.
chain/exchange/types/v2/proposal.go (4)
771-772: LGTM! Chainlink Data Streams oracle type support added.The addition of
oracletypes.OracleType_ChainlinkDataStreamsto the valid oracle types inOracleParams.ValidateBasic()enables support for the new Chainlink Data Streams oracle, which is the core objective of this PR.
1894-1903: LGTM!AdminInfo.ValidateBasic()implementation is correct.The validation properly:
- Validates the admin address using
sdk.AccAddressFromBech32only when non-empty- Checks
AdminPermissionsagainsttypes.MaxPermto ensure valid permission flags- Uses appropriate error types from the exchange module
452-456: LGTM! ConsistentAdminInfo.ValidateBasic()integration across proposals.The
AdminInfovalidation is now consistently applied acrossSpotMarketLaunchProposal,PerpetualMarketLaunchProposal, andExpiryFuturesMarketLaunchProposal, ensuring proper admin address and permission validation when provided.Also applies to: 918-922, 1031-1035
1686-1706: LGTM! Constructor properly initializes all fields.The
NewBinaryOptionsMarketParamUpdateProposalconstructor correctly accepts and assigns thehasDisabledMinimalProtocolFeeparameter along with all other fields.chain/exchange/types/v2/derivative.go (3)
14-41: LGTM! Order comparison logic is correct.
GetIsOrderLesscorrectly computes the distance from reference price for both buy and sell orders, and the sorting direction is properly controlled byisSortingFromWorstToBest.
749-765: LGTM! VWAP calculation handles edge cases properly.
ApplyExecutioncorrectly guards against nil values and zero quantity, preventing division by zero when computing the weighted average price.
978-1064: LGTM! Batch event construction with proper delta application.
ApplyDeltasAndGetDerivativeOrderBatchEventcorrectly applies deposit deltas, trading reward points, and constructs trade logs with proper fee calculations including self-relayed trade handling.injective_data/ofac.json (1)
36-36: Verify OFAC addresses against official SDN list.Three new Ethereum addresses have been added (lines 36, 62, 72). The addresses are validly formatted and the JSON structure is correct. However, confirm these addresses are from the official OFAC SDN (Specially Designated Nationals) list, as sanctions lists are compliance-critical.
Also applies to: 62-62, 72-72
| "exchange/v2/AtomicMarketOrderFeeMultiplierScheduleProposal", | ||
| nil, | ||
| ) | ||
| cdc.RegisterConcrete(&DenomMinNotionalProposal{}, "exchange/v2/DenomMinNotionalProposal", nil) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "codec.go" -path "*/exchange/types/v2/*" 2>/dev/null | head -20Repository: InjectiveLabs/sdk-go
Length of output: 98
🏁 Script executed:
cat -n ./chain/exchange/types/v2/codec.go | sed -n '1,220p'Repository: InjectiveLabs/sdk-go
Length of output: 13410
Register DenomMinNotionalProposal in govtypes.Content interface registry.
DenomMinNotionalProposal is registered in the Amino codec (line 104) but missing from the govtypes.Content interface implementations in RegisterInterfaces (lines 183-202). All other proposal types are consistently registered in both registries. Add it to maintain pattern consistency.
🤖 Prompt for AI Agents
In chain/exchange/types/v2/codec.go around line 104 and in the
RegisterInterfaces block at lines ~183-202, the DenomMinNotionalProposal type is
registered in the Amino codec but not added to the govtypes.Content interface
registrations; add DenomMinNotionalProposal to the RegisterInterfaces call so it
implements govtypes.Content alongside the other proposal types (place it with
the other proposal registrations in the same slice/map used for
govtypes.Content).
| notional := order.Fillable.Mul(order.Price) | ||
| fee := notional.Mul(positiveMakerFeePart) | ||
| remainingMargin := order.Margin.Mul(order.Fillable).Quo(order.Quantity) | ||
| cumulativeQuoteAmount = cumulativeQuoteAmount.Add(remainingMargin).Add(fee) |
There was a problem hiding this comment.
Potential division by zero when order.Quantity is zero.
If order.Quantity is zero, the calculation order.Margin.Mul(order.Fillable).Quo(order.Quantity) will panic. Consider adding a guard or ensuring orders with zero quantity are filtered upstream.
🔎 Proposed fix
+ if order.Quantity.IsZero() {
+ continue
+ }
+
notional := order.Fillable.Mul(order.Price)
fee := notional.Mul(positiveMakerFeePart)
remainingMargin := order.Margin.Mul(order.Fillable).Quo(order.Quantity)📝 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.
| notional := order.Fillable.Mul(order.Price) | |
| fee := notional.Mul(positiveMakerFeePart) | |
| remainingMargin := order.Margin.Mul(order.Fillable).Quo(order.Quantity) | |
| cumulativeQuoteAmount = cumulativeQuoteAmount.Add(remainingMargin).Add(fee) | |
| if order.Quantity.IsZero() { | |
| continue | |
| } | |
| notional := order.Fillable.Mul(order.Price) | |
| fee := notional.Mul(positiveMakerFeePart) | |
| remainingMargin := order.Margin.Mul(order.Fillable).Quo(order.Quantity) | |
| cumulativeQuoteAmount = cumulativeQuoteAmount.Add(remainingMargin).Add(fee) |
🤖 Prompt for AI Agents
In chain/exchange/types/v2/derivative.go around lines 77-80, the expression
order.Margin.Mul(order.Fillable).Quo(order.Quantity) can panic if order.Quantity
is zero; add a guard before performing the division (e.g., if
order.Quantity.IsZero() then handle by skipping this order, setting
remainingMargin to zero, or returning an error), or validate/filter out
zero-quantity orders upstream and document the assumption; ensure the chosen
behavior is explicit and covered by a unit test.
| // ComputeSyntheticVwapUnitDelta returns (price - markPrice) / markPrice | ||
| func (p *DerivativeVwapInfo) ComputeSyntheticVwapUnitDelta(marketID common.Hash) math.LegacyDec { | ||
| vwapInfo := p.PerpetualVwapInfo[marketID] | ||
| return vwapInfo.VwapData.Price.Sub(*vwapInfo.MarkPrice).Quo(*vwapInfo.MarkPrice) | ||
| } |
There was a problem hiding this comment.
Potential division by zero in ComputeSyntheticVwapUnitDelta.
If MarkPrice is nil or zero, the division vwapInfo.VwapData.Price.Sub(*vwapInfo.MarkPrice).Quo(*vwapInfo.MarkPrice) will panic. Additionally, if the marketID is not found in PerpetualVwapInfo, dereferencing vwapInfo will cause a nil pointer dereference.
🔎 Proposed fix with nil and zero checks
func (p *DerivativeVwapInfo) ComputeSyntheticVwapUnitDelta(marketID common.Hash) math.LegacyDec {
vwapInfo := p.PerpetualVwapInfo[marketID]
+ if vwapInfo == nil || vwapInfo.MarkPrice == nil || vwapInfo.MarkPrice.IsZero() {
+ return math.LegacyZeroDec()
+ }
return vwapInfo.VwapData.Price.Sub(*vwapInfo.MarkPrice).Quo(*vwapInfo.MarkPrice)
}| func (info *FeeDiscountStakingInfo) GetSortedNewFeeDiscountAccountTiers() []*AccountTierTTL { | ||
| accountTiers := make([]*AccountTierTTL, 0, len(info.AccountFeeTiers)) | ||
| info.NewAccountsMux.RLock() | ||
| for k, v := range info.NewAccounts { | ||
| accountTiers = append(accountTiers, &AccountTierTTL{ | ||
| Account: sdk.AccAddress([]byte(string(k[:]))), | ||
| TierTTL: v, | ||
| }) | ||
| } | ||
| info.NewAccountsMux.RUnlock() | ||
| sort.SliceStable(accountTiers, func(i, j int) bool { | ||
| return bytes.Compare(accountTiers[i].Account.Bytes(), accountTiers[j].Account.Bytes()) < 0 | ||
| }) | ||
| return accountTiers | ||
| } |
There was a problem hiding this comment.
Incorrect slice capacity pre-allocation.
Line 241 uses len(info.AccountFeeTiers) as capacity, but the loop iterates over info.NewAccounts. This may cause unnecessary reallocations if the maps have different sizes.
🔎 Proposed fix
func (info *FeeDiscountStakingInfo) GetSortedNewFeeDiscountAccountTiers() []*AccountTierTTL {
- accountTiers := make([]*AccountTierTTL, 0, len(info.AccountFeeTiers))
+ accountTiers := make([]*AccountTierTTL, 0, len(info.NewAccounts))
info.NewAccountsMux.RLock()
for k, v := range info.NewAccounts {🤖 Prompt for AI Agents
In chain/exchange/types/v2/fee_discounts.go around lines 240 to 254, the slice
capacity is preallocated with len(info.AccountFeeTiers) but the loop iterates
over info.NewAccounts; change the capacity to len(info.NewAccounts) (or 0 if you
prefer letting append grow) so the slice is pre-sized correctly, keep the
lock/unlock and rest of logic unchanged.
| func (info *FeeDiscountStakingInfo) GetSortedAccountVolumeContributions() []*AccountContribution { | ||
| accountContributions := make([]*AccountContribution, 0, len(info.AccountFeeTiers)) | ||
| info.AccountVolumesMux.RLock() | ||
| for k, v := range info.AccountVolumeContributions { | ||
| accountContributions = append(accountContributions, &AccountContribution{ | ||
| // use copy of value in closure, since the memory is not copied, it's reused. | ||
| // So if your closure captures it, instead of copying via call args, you'll get same index in all goroutines | ||
| Account: sdk.AccAddress([]byte(string(k[:]))), | ||
| Amount: v, | ||
| }) | ||
| } | ||
| info.AccountVolumesMux.RUnlock() | ||
| sort.SliceStable(accountContributions, func(i, j int) bool { | ||
| return bytes.Compare(accountContributions[i].Account.Bytes(), accountContributions[j].Account.Bytes()) < 0 | ||
| }) | ||
| return accountContributions | ||
| } |
There was a problem hiding this comment.
Incorrect slice capacity pre-allocation.
Line 257 uses len(info.AccountFeeTiers) as capacity, but the loop iterates over info.AccountVolumeContributions.
🔎 Proposed fix
func (info *FeeDiscountStakingInfo) GetSortedAccountVolumeContributions() []*AccountContribution {
- accountContributions := make([]*AccountContribution, 0, len(info.AccountFeeTiers))
+ accountContributions := make([]*AccountContribution, 0, len(info.AccountVolumeContributions))
info.AccountVolumesMux.RLock()
for k, v := range info.AccountVolumeContributions {📝 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 (info *FeeDiscountStakingInfo) GetSortedAccountVolumeContributions() []*AccountContribution { | |
| accountContributions := make([]*AccountContribution, 0, len(info.AccountFeeTiers)) | |
| info.AccountVolumesMux.RLock() | |
| for k, v := range info.AccountVolumeContributions { | |
| accountContributions = append(accountContributions, &AccountContribution{ | |
| // use copy of value in closure, since the memory is not copied, it's reused. | |
| // So if your closure captures it, instead of copying via call args, you'll get same index in all goroutines | |
| Account: sdk.AccAddress([]byte(string(k[:]))), | |
| Amount: v, | |
| }) | |
| } | |
| info.AccountVolumesMux.RUnlock() | |
| sort.SliceStable(accountContributions, func(i, j int) bool { | |
| return bytes.Compare(accountContributions[i].Account.Bytes(), accountContributions[j].Account.Bytes()) < 0 | |
| }) | |
| return accountContributions | |
| } | |
| func (info *FeeDiscountStakingInfo) GetSortedAccountVolumeContributions() []*AccountContribution { | |
| accountContributions := make([]*AccountContribution, 0, len(info.AccountVolumeContributions)) | |
| info.AccountVolumesMux.RLock() | |
| for k, v := range info.AccountVolumeContributions { | |
| accountContributions = append(accountContributions, &AccountContribution{ | |
| // use copy of value in closure, since the memory is not copied, it's reused. | |
| // So if your closure captures it, instead of copying via call args, you'll get same index in all goroutines | |
| Account: sdk.AccAddress([]byte(string(k[:]))), | |
| Amount: v, | |
| }) | |
| } | |
| info.AccountVolumesMux.RUnlock() | |
| sort.SliceStable(accountContributions, func(i, j int) bool { | |
| return bytes.Compare(accountContributions[i].Account.Bytes(), accountContributions[j].Account.Bytes()) < 0 | |
| }) | |
| return accountContributions | |
| } |
🤖 Prompt for AI Agents
In chain/exchange/types/v2/fee_discounts.go around lines 256-272, the slice is
preallocated with len(info.AccountFeeTiers) but the loop iterates over
info.AccountVolumeContributions; change the capacity to
len(info.AccountVolumeContributions) when calling make so the slice is correctly
sized for the data being appended.
| rpc SubaccountPositions(QuerySubaccountPositionsRequest) | ||
| returns (QuerySubaccountPositionsResponse) { | ||
| option (google.api.http).get = | ||
| "/injective/exchange/v1beta1/positions/{subaccount_id}"; | ||
| "/injective/exchange/v1beta1/subaccount_positions/{subaccount_id}"; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for references to the old HTTP path pattern
echo "=== Searching for old path pattern (positions/{subaccount_id}) ==="
rg "positions/\{subaccount_id\}" -C2
echo -e "\n=== Searching for alternative path patterns ==="
rg "positions.*subaccount_id" -C2
echo -e "\n=== Searching for SubaccountPositions references ==="
rg "SubaccountPositions" -C3
echo -e "\n=== Checking for HTTP path definitions in query.proto ==="
rg "google.api.http" proto/injective/exchange/v1beta1/query.proto -C2Repository: InjectiveLabs/sdk-go
Length of output: 50376
API endpoint path clarified, but breaking change for REST API clients.
The updated path /injective/exchange/v1beta1/subaccount_positions/{subaccount_id} is more explicit and aligns better with the SubaccountPositions method name, distinguishing it from the general Positions endpoint.
This is a breaking change for external REST API clients directly calling this endpoint. However, SDK users relying on the generated gRPC client method (FetchChainSubaccountPositions) are unaffected since they use the RPC layer, not hardcoded HTTP paths.
Ensure that REST API clients and downstream services relying on direct HTTP calls are updated with the new path.
🤖 Prompt for AI Agents
In proto/injective/exchange/v1beta1/query.proto around lines 259-263, the HTTP
path for SubaccountPositions was changed to
/injective/exchange/v1beta1/subaccount_positions/{subaccount_id}, which breaks
existing REST clients; to fix this without breaking clients, add an additional
HTTP binding that preserves the old path (using
google.api.http.additional_bindings) mapping the previous endpoint to the same
RPC, or if you intend to break compatibility, update the API docs/changelog and
notify downstreams and SDK maintainers to replace the old REST URL with the new
one.
| // Retrieves the state for all Chainlink Data Streamsprice feeds | ||
| rpc ChainlinkDataStreamsPriceStates( | ||
| QueryChainlinkDataStreamsPriceStatesRequest) | ||
| returns (QueryChainlinkDataStreamsPriceStatesResponse) { | ||
| option (google.api.http).get = | ||
| "/injective/oracle/v1beta1/chainlink_data_stream_price_states"; | ||
| } |
There was a problem hiding this comment.
Minor typo in comment and HTTP path inconsistency.
- Line 79: "Streamsprice" should be "Streams price" (missing space)
- HTTP path uses singular
chainlink_data_stream_price_stateswhile the RPC/message names use pluralChainlinkDataStreams- consider aligning for consistency (e.g.,chainlink_data_streams_price_states)
🔎 Suggested fix
- // Retrieves the state for all Chainlink Data Streamsprice feeds
+ // Retrieves the state for all Chainlink Data Streams price feeds
rpc ChainlinkDataStreamsPriceStates(
QueryChainlinkDataStreamsPriceStatesRequest)
returns (QueryChainlinkDataStreamsPriceStatesResponse) {
option (google.api.http).get =
- "/injective/oracle/v1beta1/chainlink_data_stream_price_states";
+ "/injective/oracle/v1beta1/chainlink_data_streams_price_states";
}🤖 Prompt for AI Agents
In proto/injective/oracle/v1beta1/query.proto around lines 79 to 85, fix the
comment typo by changing "Streamsprice" to "Streams price" and make the HTTP
path consistent with the RPC/message pluralization by renaming the option path
from "/injective/oracle/v1beta1/chainlink_data_stream_price_states" to
"/injective/oracle/v1beta1/chainlink_data_streams_price_states"; also search for
and update any other references to the old singular path to avoid mismatches.
Solves C-655
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.