[c-930] use metrics v2 to trace relevant code paths#84
[c-930] use metrics v2 to trace relevant code paths#84dbrajovic wants to merge 6 commits intov0.50.x-injfrom
Conversation
|
@dbrajovic your pull request is missing a changelog! |
📝 WalkthroughWalkthroughThis PR adds metrics instrumentation across the codebase: modules gain a Changes
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
go.mod (1)
64-64:⚠️ Potential issue | 🔴 CriticalCritical: Pipeline failing due to confirmed vulnerable dependencies.
The following confirmed vulnerabilities must be addressed before merging:
google.golang.org/grpc@1.75.0(Critical - GHSA-p77j-4mvh-x3m3): Authorization bypass via missing leading slash in:pathheader. Upgrade to v1.79.3 or later.
go.opentelemetry.io/otel/sdk@1.38.0(High - GHSA-9h8m-3fm2-qjrq): Arbitrary code execution via PATH hijacking on macOS/Darwin systems. Upgrade to v1.40.0 or later.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` at line 64, Update the vulnerable module versions in go.mod: bump google.golang.org/grpc to v1.79.3 or later and go.opentelemetry.io/otel/sdk to v1.40.0 or later, then run dependency resolution (e.g., use go get google.golang.org/grpc@v1.79.3 and go get go.opentelemetry.io/otel/sdk@v1.40.0) and go mod tidy to ensure the upgraded transitive deps are resolved; verify the updates affect any references to these modules in build/test pipelines and re-run CI to confirm the vulnerabilities are resolved.x/slashing/keeper/signing_info.go (2)
261-289:⚠️ Potential issue | 🟠 MajorReturn immediately when the callback asks to stop.
The contract says iteration halts when the callback returns true, but Lines 283-285 only break the inner bit loop. The outer chunk loop keeps running, so callers can still receive more callbacks after requesting stop.
Suggested fix
- if cb(index, bs.Test(i)) { - break - } + if cb(index, bs.Test(i)) { + return nil + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/slashing/keeper/signing_info.go` around lines 261 - 289, IterateMissedBlockBitmap isn't stopping the outer chunk loop when the callback cb(index, ...) returns true — change the control flow so that when cb returns true you return immediately (e.g., return nil) instead of breaking only the inner bit loop; update the inner loop in IterateMissedBlockBitmap to return upon cb true so the function exits and no further chunks/callbacks are processed, keeping the existing deferred iter.Close and error handling intact.
197-230:⚠️ Potential issue | 🟠 MajorPropagate the bitmap write error.
Line 229 ignores the result of
setMissedBlockBitmapChunk. A failed store write currently returnsnil, so missed-block state can silently diverge from slashing state.Suggested fix
- k.setMissedBlockBitmapChunk(sdkCtx, addr, chunkIndex, updatedChunk) - return nil + if err := k.setMissedBlockBitmapChunk(sdkCtx, addr, chunkIndex, updatedChunk); err != nil { + return errors.Wrapf(err, "failed to persist bitmap chunk; index: %d", index) + } + return nil🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/slashing/keeper/signing_info.go` around lines 197 - 230, In SetMissedBlockBitmapValue, the return value from k.setMissedBlockBitmapChunk(sdkCtx, addr, chunkIndex, updatedChunk) is ignored so storage errors are dropped; capture its error, wrap it (e.g., with context like "failed to set bitmap chunk; index: %d") and return it from SetMissedBlockBitmapValue so write failures propagate to the caller (referencing the function SetMissedBlockBitmapValue and the method setMissedBlockBitmapChunk).x/staking/keeper/validator.go (1)
329-351:⚠️ Potential issue | 🟠 MajorClose both iterators before returning.
GetValidatorsandIterateLastValidatorPowerseach open a store iterator but never close it. Any decode failure, handler stop, or normal return leaks the iterator.Suggested fix
iterator, err := store.Iterator(types.ValidatorsKey, storetypes.PrefixEndBytes(types.ValidatorsKey)) if err != nil { return nil, err } + defer iterator.Close() @@ iter, err := k.LastValidatorsIterator(sdkCtx) if err != nil { return err } + defer iter.Close()Also applies to: 453-475
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/staking/keeper/validator.go` around lines 329 - 351, GetValidators (and similarly IterateLastValidatorPowers) opens a KV iterator that is never closed, leaking resources on errors or normal returns; fix by ensuring the iterator is closed on all paths (use a defer iterator.Close() immediately after successful store.Iterator(...) return, and ensure any early returns still run the defer) for functions GetValidators and IterateLastValidatorPowers so the iterator is always released even when UnmarshalValidator or other steps return an error.
🟠 Major comments (18)
x/crisis/keeper/keeper.go-21-21 (1)
21-21:⚠️ Potential issue | 🟠 MajorGuard lazy meter initialization against concurrent access.
k.meteris mutated in the hot path without synchronization. Concurrent calls can race on first initialization and produce undefined behavior under race conditions.Proposed fix
import ( "context" "fmt" + "sync" "time" @@ type Keeper struct { meter metrics.Meter + meterOnce sync.Once routes []types.InvarRoute @@ func (k *Keeper) Meter(ctx context.Context) metrics.Meter { - if k.meter == nil { - k.meter = sdk.UnwrapSDKContext(ctx).Meter().SubMeter(types.ModuleName, metrics.Tag("svc", types.ModuleName)) - } + k.meterOnce.Do(func() { + k.meter = sdk.UnwrapSDKContext(ctx).Meter().SubMeter(types.ModuleName, metrics.Tag("svc", types.ModuleName)) + }) return k.meter }Also applies to: 138-144
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/crisis/keeper/keeper.go` at line 21, The lazy initialization of k.meter on the keeper struct is racy; add a synchronization primitive to the keeper (e.g., a sync.Mutex field or a sync.Once) and use it when checking/setting k.meter so concurrent callers cannot race on first initialization; update all places that lazily mutate k.meter (the hot-path methods that reference k.meter) to acquire the mutex or use the Once to perform initialization and then read k.meter, ensuring a double-check (nil check before and after locking if using a mutex) to avoid unnecessary locking.x/gov/keeper/keeper.go-202-208 (1)
202-208:⚠️ Potential issue | 🟠 MajorSame Meter caching issue as staking keeper.
The
Metermethod uses a pointer receiver, butLogger(line 155) uses a value receiver. This has the same ineffective caching problem described inx/staking/keeper/keeper.go—the meter will be re-created on each call from value receiver methods.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/gov/keeper/keeper.go` around lines 202 - 208, The Meter method is using a pointer receiver while Logger uses a value receiver, causing cached fields to be ineffective for value-receiver methods; change the Meter signature to use a value receiver (func (k Keeper) Meter(...)) so it matches Logger and prevents the meter from being re-created when called from value-receiver methods, keeping the caching behavior consistent for the Keeper type.x/staking/keeper/keeper.go-191-197 (1)
191-197:⚠️ Potential issue | 🟠 MajorMeter caching ineffective due to value receivers.
The
Metermethod uses a pointer receiver (*Keeper) to cache the meter, but most keeper methods that call it (e.g.,Logger,GetLastTotalPower,SetLastTotalPower) use value receivers (Keeper). When a value receiver method callsk.Meter(sdkCtx), the implicit address-of creates a copy, so the cachedk.meterassignment doesn't persist across calls. This results in creating a new SubMeter on every invocation, defeating the lazy initialization purpose.Consider either:
- Change all keeper methods to pointer receivers, or
- Initialize the meter in
NewKeeperinstead of lazy initializationProposed fix: Initialize meter in NewKeeper
func NewKeeper( cdc codec.BinaryCodec, storeService storetypes.KVStoreService, ak types.AccountKeeper, bk types.BankKeeper, authority string, validatorAddressCodec addresscodec.Codec, consensusAddressCodec addresscodec.Codec, + meter metrics.Meter, ) *Keeper { // ... existing validation ... return &Keeper{ + meter: meter, storeService: storeService, cdc: cdc, // ... rest of fields ... } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/staking/keeper/keeper.go` around lines 191 - 197, The Meter method's lazy cache on *Keeper is ineffective because many callers use value receivers (e.g., Logger, GetLastTotalPower, SetLastTotalPower) so the cached k.meter doesn't persist; fix by initializing the meter once in NewKeeper instead of lazy init: in NewKeeper compute and assign k.meter using ctx.Meter().SubMeter(...) (referencing Keeper.Meter and NewKeeper) and simplify/remove the lazy set in Meter so it just returns k.meter (or keep a nil-check that does not attempt to set); this avoids relying on pointer receiver semantics and ensures a single SubMeter instance is used.x/auth/keeper/keeper.go-305-311 (1)
305-311:⚠️ Potential issue | 🟠 MajorData race in lazy meter initialization.
The
Metermethod has a potential data race: multiple goroutines can concurrently read and writeak.meterwithout synchronization. While the functional outcome may be equivalent (all goroutines would create similar sub-meters), this is a race condition that can be detected by the Go race detector.Consider either:
- Initialize the meter in
NewAccountKeeper(if context is available at that point), or- Use
sync.Oncefor thread-safe lazy initialization.🔒 Proposed fix using sync.Once
Add a
sync.Oncefield to the struct:type AccountKeeper struct { // ... existing fields ... meter metrics.Meter meterOnce sync.Once }Then update the
Metermethod:func (ak *AccountKeeper) Meter(ctx context.Context) metrics.Meter { - if ak.meter == nil { - ak.meter = sdk.UnwrapSDKContext(ctx).Meter().SubMeter(types.ModuleName, metrics.Tag("svc", types.ModuleName)) - } - - return ak.meter + ak.meterOnce.Do(func() { + ak.meter = sdk.UnwrapSDKContext(ctx).Meter().SubMeter(types.ModuleName, metrics.Tag("svc", types.ModuleName)) + }) + return ak.meter }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/auth/keeper/keeper.go` around lines 305 - 311, The Meter method has a data race on ak.meter; add a sync.Once field (e.g. meterOnce sync.Once) to the AccountKeeper struct and change Meter to perform thread-safe lazy init via ak.meterOnce.Do(func(){ ak.meter = sdk.UnwrapSDKContext(ctx).Meter().SubMeter(types.ModuleName, metrics.Tag("svc", types.ModuleName)) }), returning ak.meter afterwards; alternatively, if a suitable context is available when constructing the keeper, initialize ak.meter in NewAccountKeeper instead. Ensure you import sync and remove unsynchronized writes to ak.meter elsewhere.x/authz/keeper/keeper.go-486-492 (1)
486-492:⚠️ Potential issue | 🟠 MajorData race in lazy meter initialization (same issue as x/auth/keeper/keeper.go).
This
Metermethod has the same race condition: concurrent goroutines can simultaneously checkk.meter == niland write to it without synchronization.Apply the same fix pattern using
sync.Once:🔒 Proposed fix using sync.Once
Add a
sync.Oncefield to the struct:type Keeper struct { storeService corestoretypes.KVStoreService cdc codec.Codec router baseapp.MessageRouter authKeeper authz.AccountKeeper bankKeeper authz.BankKeeper meter metrics.Meter + meterOnce sync.Once }Then update the
Metermethod:func (k *Keeper) Meter(ctx context.Context) metrics.Meter { - if k.meter == nil { - k.meter = sdk.UnwrapSDKContext(ctx).Meter().SubMeter(authz.ModuleName, metrics.Tag("svc", authz.ModuleName)) - } - - return k.meter + k.meterOnce.Do(func() { + k.meter = sdk.UnwrapSDKContext(ctx).Meter().SubMeter(authz.ModuleName, metrics.Tag("svc", authz.ModuleName)) + }) + return k.meter }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/authz/keeper/keeper.go` around lines 486 - 492, The Meter method on Keeper has a data race when lazily initializing k.meter; add a sync.Once field (e.g., meterOnce) to the Keeper struct and use meterOnce.Do(...) inside Keeper.Meter to perform the one-time initialization using sdk.UnwrapSDKContext(ctx).Meter().SubMeter(authz.ModuleName, metrics.Tag("svc", authz.ModuleName)), leaving the method to simply return k.meter after the Do call; follow the same pattern used in x/auth/keeper/keeper.go to ensure thread-safe lazy init.x/nft/keeper/keeper.go-17-18 (1)
17-18:⚠️ Potential issue | 🟠 MajorRemove the mutable
meterfield and eliminate lazy initialization to prevent cache corruption and race conditions.The
Metermethod uses a pointer receiver while all other keeper methods use value receivers. When value-receiver methods callk.Meter(), Go implicitly creates a temporary pointer to the copy (&k), and any mutations tok.meterare lost when the method returns. This causes:
- Receiver-copy loss: The lazy-initialized cache is never persisted back to the original keeper.
- Race condition: Concurrent calls to
Meter()from different value copies race to initializek.meteron separate copies.Recommended fix: Remove mutable cache
type Keeper struct { - meter metrics.Meter cdc codec.BinaryCodec storeService store.KVStoreService bk nft.BankKeeper ac address.Codec } @@ -func (k *Keeper) Meter(ctx context.Context) metrics.Meter { - if k.meter == nil { - k.meter = sdk.UnwrapSDKContext(ctx).Meter().SubMeter(nft.ModuleName, metrics.Tag("svc", nft.ModuleName)) - } - - return k.meter +func (k Keeper) Meter(ctx context.Context) metrics.Meter { + return sdk.UnwrapSDKContext(ctx).Meter().SubMeter(nft.ModuleName, metrics.Tag("svc", nft.ModuleName)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/nft/keeper/keeper.go` around lines 17 - 18, Remove the mutable meter field from the keeper struct and stop lazy-initializing it in Meter; instead make Meter a non-mutating accessor that returns a fresh or globally initialized metrics.Meter without writing to k.meter. Concretely: delete the meter field declaration, change Meter to use a metrics provider/factory (or a package-level singleton initialized with sync.Once in an init/path-safe place) and return that meter directly from Meter (keep Meter as a value receiver and do not assign to any keeper fields). Update any references to k.meter to call k.Meter() and ensure no code mutates the keeper inside Meter to avoid the receiver-copy loss and race conditions.x/distribution/keeper/validator.go-181-181 (1)
181-181:⚠️ Potential issue | 🟠 MajorPropagate
incrementReferenceCountfailure.
Line 181drops the returned error. If the reference-count update fails, execution still writes slash state, which can leave historical rewards tracking inconsistent.💡 Proposed fix
- k.incrementReferenceCount(sdkCtx, valAddr, newPeriod) + if err := k.incrementReferenceCount(sdkCtx, valAddr, newPeriod); err != nil { + return err + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/distribution/keeper/validator.go` at line 181, The call to k.incrementReferenceCount(sdkCtx, valAddr, newPeriod) currently ignores its returned error; update the caller (the function containing that call) to check the returned error and propagate it (return the error) instead of dropping it so execution halts when the reference-count update fails and avoids writing slash state inconsistently. Locate the incrementReferenceCount(...) call in validator.go and change the call to capture its error (err := k.incrementReferenceCount(...)); if err != nil { return err } (or wrap and return the error consistent with the surrounding function's error handling).x/evidence/keeper/grpc_query.go-48-50 (1)
48-50:⚠️ Potential issue | 🟠 MajorHandle
Evidences.Geterrors instead of discarding them.At line 48, the returned error is ignored, which converts all failures—including storage and codec errors—into
codes.NotFound. Other call sites in the module properly check this error (seekeeper.go:107andgenesis.go:26).The proposed fix captures the error and checks it. Note that if the error is not
collections.ErrNotFound, it should return an appropriate error code (e.g.,codes.Internal) rather thancodes.NotFoundto accurately reflect the failure type.💡 Proposed fix
- evidence, _ := k.k.Evidences.Get(sdkCtx, decodedHash) - if evidence == nil { + evidence, err := k.k.Evidences.Get(sdkCtx, decodedHash) + if err != nil { + return nil, status.Errorf(codes.NotFound, "evidence %s not found", req.Hash) + } + if evidence == nil { return nil, status.Errorf(codes.NotFound, "evidence %s not found", req.Hash) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/evidence/keeper/grpc_query.go` around lines 48 - 50, The call to k.k.Evidences.Get currently discards the returned error which causes all failures to be reported as codes.NotFound; change the code in the handler that calls k.k.Evidences.Get(sdkCtx, decodedHash) to capture the error (e.g., evidence, err := k.k.Evidences.Get(...)), check if err != nil and if errors.Is(err, collections.ErrNotFound) return status.Errorf(codes.NotFound, ...), otherwise return status.Errorf(codes.Internal, "failed to get evidence: %v", err); keep the existing nil-evidence check but only treat it as NotFound when the collection error indicates absence.x/evidence/keeper/keeper.go-128-134 (1)
128-134:⚠️ Potential issue | 🟠 MajorData race on lazy meter initialization.
The
Meter()method has a race condition when accessed concurrently. Multiple goroutines can simultaneously observek.meter == nil, causing redundant SubMeter allocations and potential data races on field assignment.This pattern is systemic—identical implementations exist across 22 keepers in the codebase (bank, auth, staking, gov, feegrant, slashing, etc.). If fixing this in evidence keeper, consider applying the same fix across all affected keepers.
Proposed fix using sync.Once
Add a
sync.Oncefield toKeeper:type Keeper struct { + meterOnce sync.Once meter metrics.Meter cdc codec.BinaryCodecUpdate the
Metermethod:func (k *Keeper) Meter(ctx context.Context) metrics.Meter { - if k.meter == nil { - k.meter = sdk.UnwrapSDKContext(ctx).Meter().SubMeter(types.ModuleName, metrics.Tag("svc", types.ModuleName)) - } - + k.meterOnce.Do(func() { + k.meter = sdk.UnwrapSDKContext(ctx).Meter().SubMeter(types.ModuleName, metrics.Tag("svc", types.ModuleName)) + }) return k.meter }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/evidence/keeper/keeper.go` around lines 128 - 134, The Meter method has a race on lazy initialization of k.meter; add a sync.Once field (e.g., meterOnce sync.Once) to the Keeper struct and change Meter(ctx context.Context) to initialize k.meter inside meterOnce.Do(func() { ... }) so only one goroutine calls sdk.UnwrapSDKContext(ctx).Meter().SubMeter(...) and assigns k.meter; update evidence Keeper and apply the same pattern to other keepers with identical lazy initialization (bank, auth, staking, gov, feegrant, slashing, etc.).x/circuit/keeper/keeper.go-18-19 (1)
18-19:⚠️ Potential issue | 🟠 Major
Meter()has a race-prone lazy init on shared keeper state.
k.meteris written without synchronization on lines 85–86. Concurrent access from query handlers, message handlers, and genesis operations can race. Usesync.Onceto safely initialize.Suggested fix
import ( context "context" + "sync" @@ type Keeper struct { meter metrics.Meter + meterOnce sync.Once cdc codec.BinaryCodec @@ func (k *Keeper) Meter(ctx context.Context) metrics.Meter { - if k.meter == nil { - k.meter = sdk.UnwrapSDKContext(ctx).Meter().SubMeter(types.ModuleName, metrics.Tag("svc", types.ModuleName)) - } + k.meterOnce.Do(func() { + k.meter = sdk.UnwrapSDKContext(ctx).Meter().SubMeter(types.ModuleName, metrics.Tag("svc", types.ModuleName)) + }) return k.meter }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/circuit/keeper/keeper.go` around lines 18 - 19, The Meter() method currently lazily assigns k.meter without synchronization, causing races; add a sync.Once field (e.g., meterOnce sync.Once) to the keeper struct and change Meter() to initialize k.meter inside meterOnce.Do(func(){ ... }) so the assignment to k.meter happens exactly once; also add the sync import and keep the rest of Meter() behavior the same (returning k.meter) to ensure thread-safe lazy initialization.x/feegrant/keeper/msg_server.go-47-48 (1)
47-48:⚠️ Potential issue | 🟠 MajorDon't ignore
GetAllowancefailures.Line 47 drops the read error and treats it as "no allowance". If that lookup fails because the stored grant is unreadable, this handler can still proceed as if nothing exists.
Suggested fix
- if f, _ := k.GetAllowance(sdkCtx, granter, grantee); f != nil { + if f, err := k.GetAllowance(sdkCtx, granter, grantee); err != nil { + return nil, err + } else if f != nil { return nil, errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "fee allowance already exists") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/feegrant/keeper/msg_server.go` around lines 47 - 48, Replace the ignored-error call to GetAllowance with an explicit check: call f, err := k.GetAllowance(sdkCtx, granter, grantee); if err != nil return nil, errorsmod.Wrap(err, "failed to read existing allowance") to propagate storage/read errors, then if f != nil return nil, errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "fee allowance already exists"); this ensures GetAllowance failures are not treated as “no allowance” and are surfaced correctly.x/staking/keeper/delegation.go-947-954 (1)
947-954:⚠️ Potential issue | 🟠 MajorBug:
currTimeparameter is ignored, usingsdkCtx.HeaderInfo().Timeinstead.The function accepts
currTime time.Timeas a parameter but line 954 passessdkCtx.HeaderInfo().TimetoRedelegationQueueIteratorinstead of the providedcurrTime. This could cause incorrect behavior if a caller passes a different time value.🐛 Proposed fix
// gets an iterator for all timeslices from time 0 until the current Blockheader time - redelegationTimesliceIterator, err := k.RedelegationQueueIterator(sdkCtx, sdkCtx.HeaderInfo().Time) + redelegationTimesliceIterator, err := k.RedelegationQueueIterator(sdkCtx, currTime)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/staking/keeper/delegation.go` around lines 947 - 954, The function DequeueAllMatureRedelegationQueue currently ignores its currTime parameter and calls RedelegationQueueIterator with sdkCtx.HeaderInfo().Time; change the call to use the provided currTime so the iterator covers times up to the passed-in value (i.e., replace sdkCtx.HeaderInfo().Time with currTime when invoking RedelegationQueueIterator and ensure any subsequent logic that relies on the iteration uses currTime consistently).x/distribution/keeper/keeper.go-237-243 (1)
237-243:⚠️ Potential issue | 🟠 MajorRace condition in lazy initialization of
meterfield.The
Metermethod has a check-then-write race condition. If multiple goroutines callMeter()concurrently whenk.meter == nil, they can both pass the nil check and create separate meter instances. While this may not cause crashes, it leads to inconsistent meter state.Consider using
sync.Oncefor thread-safe initialization:🔒 Proposed fix using sync.Once
Add a
sync.Oncefield to the Keeper struct:type Keeper struct { // ... existing fields ... meter metrics.Meter + meterOnce sync.Once }Then update the Meter method:
func (k *Keeper) Meter(ctx context.Context) metrics.Meter { - if k.meter == nil { - k.meter = sdk.UnwrapSDKContext(ctx).Meter().SubMeter(types.ModuleName, metrics.Tag("svc", types.ModuleName)) - } - - return k.meter + k.meterOnce.Do(func() { + k.meter = sdk.UnwrapSDKContext(ctx).Meter().SubMeter(types.ModuleName, metrics.Tag("svc", types.ModuleName)) + }) + return k.meter }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/distribution/keeper/keeper.go` around lines 237 - 243, The lazy initialization of k.meter in Keeper.Meter has a race: multiple goroutines can pass the nil check and create separate meters; add a sync.Once field (e.g., meterOnce) to the Keeper struct and move the initialization into meterOnce.Do to ensure the creation of k.meter happens exactly once; update Meter(ctx context.Context) to call k.meterOnce.Do(func(){ k.meter = sdk.UnwrapSDKContext(ctx).Meter().SubMeter(types.ModuleName, metrics.Tag("svc", types.ModuleName)) }) and then return k.meter.x/bank/keeper/msg_server.go-38-42 (1)
38-42:⚠️ Potential issue | 🟠 Major
NewMsgServerImpladvertises aKeeperinterface but all four message handlers require the concreteBaseKeepertype.The constructor accepts
keeper Keeperat the API level, yetSend,MultiSend,UpdateParams, andSetSendEnabledeach assertk.Keeper.(BaseKeeper)and fail at runtime if the dynamic type is notBaseKeeper. This breaks the interface contract: any implementation ofKeeperother thanBaseKeeperwill be rejected at runtime instead of at the type-check level.Either tighten the constructor and field types to
BaseKeeperto make the requirement explicit, or expose the required metering and address-codec methods on theKeeperinterface instead of asserting the concrete type.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/bank/keeper/msg_server.go` around lines 38 - 42, NewMsgServerImpl's constructor and its msg server type take a generic Keeper but every handler (Send, MultiSend, UpdateParams, SetSendEnabled) asserts k.Keeper.(BaseKeeper), causing runtime failures; either change the API to require BaseKeeper explicitly or add the needed methods to the Keeper interface. Fix by updating NewMsgServerImpl to accept and store BaseKeeper (replace Keeper with BaseKeeper in the constructor and the msg server struct) so the metering and address-codec methods are available without type assertions, or alternatively extend the Keeper interface to declare the metering and address codec methods used by base.Meter(...) and other handlers and remove the concrete BaseKeeper assertions. Ensure all references to k.Keeper and the constructor signature (NewMsgServerImpl) and usages in Send, MultiSend, UpdateParams, SetSendEnabled are updated accordingly.x/feegrant/keeper/keeper.go-25-25 (1)
25-25:⚠️ Potential issue | 🟠 MajorValue-receiver methods calling pointer-receiver Meter() break the lazy cache.
Line 421 implements a pointer-receiver
Meter()that mutatesk.meterat line 423, but all methods in this file are value receivers (func (k Keeper)). When a value-receiver method callsk.Meter(), the assignment mutates a temporary copy, so the sub-meter is recreated on every invocation instead of being cached. Either switch all metered keeper methods to pointer receivers, or makeMeterstateless by deriving the sub-meter directly fromsdkCtxwithout caching.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/feegrant/keeper/keeper.go` at line 25, The keeper currently caches a sub-meter in the Keeper.meter field via the pointer-receiver Meter() (which assigns k.meter), but all keeper methods in this file use value receivers (func (k Keeper)), so calls to k.Meter() mutate a copy and the cache is lost; to fix either: (A) switch all keeper methods in this file from value receivers to pointer receivers (change func (k Keeper) ... to func (k *Keeper) ...) so k.Meter() mutates the real Keeper and the cached k.meter is reused, or (B) make Meter() stateless by removing the k.meter cache and deriving/returning the sub-meter directly from the sdk.Context (or relevant sdkCtx) inside Meter() so it no longer mutates Keeper; update references to Meter(), Keeper.meter, and any colocated methods accordingly.x/slashing/keeper/keeper.go-21-21 (1)
21-21:⚠️ Potential issue | 🟠 MajorThe lazy meter cache is ineffective with value receivers calling pointer receiver method.
Value receiver methods (Logger, AddPubkey, GetPubkey, Slash, SlashWithInfractionReason, Jail, deleteAddrPubkeyRelation) call
k.Meter(), which is a pointer receiver. Each value receiver call creates a struct copy, so the pointer receiver's lazy initialization writes to the copy's meter field, which is discarded after the call. On the next invocation, a new copy is created and the cache is rebuilt. Either make all keeper methods pointer receivers or drop the struct-level cache.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/slashing/keeper/keeper.go` at line 21, The lazy meter cache on the keeper is broken because methods like Logger, AddPubkey, GetPubkey, Slash, SlashWithInfractionReason, Jail, and deleteAddrPubkeyRelation use value receivers but call the pointer-receiver Meter(), causing the cached meter to be written into a copy; fix by making these methods use pointer receivers (change their receiver from k keeper to k *keeper) so Meter() writes to the real struct, or alternatively remove the struct-level meter cache and compute/return the meter directly in Meter(); update all affected method signatures (Logger, AddPubkey, GetPubkey, Slash, SlashWithInfractionReason, Jail, deleteAddrPubkeyRelation) and any call sites accordingly to eliminate the copy bug.x/gov/keeper/msg_server.go-314-317 (1)
314-317:⚠️ Potential issue | 🟠 MajorUse distinct timing names for the legacy wrappers.
These methods immediately delegate to the v1 server, and the keeper-backed v1 implementation already records
SubmitProposal,Vote,VoteWeighted, andDeposit. Reusing the same metric names here double-counts every legacy request and skews latency/throughput dashboards.Suggested direction
- defer k.keeper.Meter(sdkCtx).FuncTiming(&sdkCtx, "SubmitProposal")() + defer k.keeper.Meter(sdkCtx).FuncTiming(&sdkCtx, "LegacySubmitProposal")() - defer k.keeper.Meter(sdkCtx).FuncTiming(&sdkCtx, "Vote")() + defer k.keeper.Meter(sdkCtx).FuncTiming(&sdkCtx, "LegacyVote")() - defer k.keeper.Meter(sdkCtx).FuncTiming(&sdkCtx, "VoteWeighted")() + defer k.keeper.Meter(sdkCtx).FuncTiming(&sdkCtx, "LegacyVoteWeighted")() - defer k.keeper.Meter(sdkCtx).FuncTiming(&sdkCtx, "Deposit")() + defer k.keeper.Meter(sdkCtx).FuncTiming(&sdkCtx, "LegacyDeposit")()Also applies to: 355-358, 370-373, 393-396
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/gov/keeper/msg_server.go` around lines 314 - 317, The legacy wrapper methods (legacyMsgServer methods SubmitProposal, Vote, VoteWeighted, Deposit) are reusing the same metric names as the keeper-backed v1 implementations causing double-counting; change the timing metric names in the legacy wrappers to distinct identifiers (for example prefix with "legacy_" or "_legacy") when calling k.keeper.Meter(sdkCtx).FuncTiming(&sdkCtx, ...) so the keeper-backed v1 metrics remain unchanged and dashboards are not skewed; update each legacy method (legacyMsgServer.SubmitProposal, legacyMsgServer.Vote, legacyMsgServer.VoteWeighted, legacyMsgServer.Deposit) to use the new unique timing string.x/upgrade/keeper/keeper.go-641-646 (1)
641-646:⚠️ Potential issue | 🟠 MajorDon't memoize
meteron a copied keeper.
Metercaches on*Keeper, but most instrumented methods in this file usefunc (k Keeper)receivers. In Go that meansk.Meter(...)writes the cache to the local copy, so the sub-meter is rebuilt on every call. The first lazy init can also race if multiple goroutines callMeter()concurrently on the same keeper. Either move the instrumented methods to pointer receivers or stop storing the meter on the keeper.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/upgrade/keeper/keeper.go` around lines 641 - 646, The Keeper.Meter method currently memoizes the meter on k.meter which is unsafe because many instrumented methods use value receivers (func (k Keeper)) so the cache is stored on copies and can race; fix by removing the cached field usage: make Meter(ctx context.Context) return sdk.UnwrapSDKContext(ctx).Meter().SubMeter(types.ModuleName, metrics.Tag("svc", types.ModuleName)) unconditionally (i.e., do not assign to k.meter), or alternatively convert the instrumented methods to pointer receivers and keep k.meter with proper synchronization; update references to Meter, the k.meter field, and any funcs with receivers (methods declared as func (k Keeper)) accordingly.
🟡 Minor comments (1)
x/nft/keeper/nft_batch.go-96-98 (1)
96-98:⚠️ Potential issue | 🟡 MinorPotentially misleading error wrapping.
When
transferWithNoCheckfails, wrapping the error withnft.ErrNFTNotExistsmay mask the actual transfer failure reason. The NFT existence is already verified on line 93, so a transfer failure would be due to a different issue.Consider returning the original error or using a more appropriate error type:
Proposed fix
- if err := k.transferWithNoCheck(sdkCtx, classID, nftID, receiver); err != nil { - return errors.Wrap(nft.ErrNFTNotExists, nftID) - } + if err := k.transferWithNoCheck(sdkCtx, classID, nftID, receiver); err != nil { + return err + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/nft/keeper/nft_batch.go` around lines 96 - 98, The current error handling in nft_batch.go wraps any error from transferWithNoCheck(sdkCtx, classID, nftID, receiver) with nft.ErrNFTNotExists which can mask the real cause; instead, return the original err or wrap the returned err with a contextual message (e.g., errors.Wrap(err, "transferWithNoCheck failed") or fmt.Errorf("transfer failed: %w", err)) so the true failure from transferWithNoCheck is preserved; update the return in the transferWithNoCheck error branch to propagate or appropriately wrap the actual err rather than nft.ErrNFTNotExists.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 727af76a-19ac-44a8-a8e6-942f1b434dd6
⛔ Files ignored due to path filters (9)
go.sumis excluded by!**/*.sumsimapp/go.sumis excluded by!**/*.sumstore/go.sumis excluded by!**/*.sumtests/go.sumis excluded by!**/*.sumx/circuit/go.sumis excluded by!**/*.sumx/evidence/go.sumis excluded by!**/*.sumx/feegrant/go.sumis excluded by!**/*.sumx/nft/go.sumis excluded by!**/*.sumx/upgrade/go.sumis excluded by!**/*.sum
📒 Files selected for processing (121)
baseapp/abci.gogo.modserver/start.gosimapp/go.modstore/go.modtests/go.modtests/integration/gov/keeper/keeper_test.gox/auth/keeper/account.gox/auth/keeper/genesis.gox/auth/keeper/grpc_query.gox/auth/keeper/keeper.gox/auth/keeper/msg_server.gox/authz/keeper/genesis.gox/authz/keeper/grpc_query.gox/authz/keeper/keeper.gox/authz/keeper/msg_server.gox/bank/keeper/genesis.gox/bank/keeper/grpc_query.gox/bank/keeper/keeper.gox/bank/keeper/msg_server.gox/bank/keeper/send.gox/bank/keeper/transient_balances.gox/bank/keeper/view.gox/bank/keeper/virtual_balances.gox/circuit/go.modx/circuit/keeper/genesis.gox/circuit/keeper/keeper.gox/circuit/keeper/msg_server.gox/circuit/keeper/query.gox/consensus/keeper/keeper.gox/crisis/keeper/genesis.gox/crisis/keeper/keeper.gox/crisis/keeper/msg_server.gox/distribution/keeper/alias_functions.gox/distribution/keeper/allocation.gox/distribution/keeper/delegation.gox/distribution/keeper/fee_pool.gox/distribution/keeper/genesis.gox/distribution/keeper/grpc_query.gox/distribution/keeper/hooks.gox/distribution/keeper/keeper.gox/distribution/keeper/msg_server.gox/distribution/keeper/params.gox/distribution/keeper/store.gox/distribution/keeper/validator.gox/evidence/go.modx/evidence/keeper/abci.gox/evidence/keeper/grpc_query.gox/evidence/keeper/infraction.gox/evidence/keeper/keeper.gox/evidence/keeper/msg_server.gox/feegrant/go.modx/feegrant/keeper/grpc_query.gox/feegrant/keeper/keeper.gox/feegrant/keeper/migrations.gox/feegrant/keeper/msg_server.gox/gov/keeper/deposit.gox/gov/keeper/grpc_query.gox/gov/keeper/keeper.gox/gov/keeper/keeper_test.gox/gov/keeper/migrations.gox/gov/keeper/msg_server.gox/gov/keeper/proposal.gox/gov/keeper/tally.gox/gov/keeper/vote.gox/gov/module.gox/group/keeper/genesis.gox/group/keeper/grpc_query.gox/group/keeper/keeper.gox/group/keeper/migrations.gox/group/keeper/msg_server.gox/group/keeper/proposal_executor.gox/group/keeper/tally.gox/mint/keeper/genesis.gox/mint/keeper/grpc_query.gox/mint/keeper/keeper.gox/mint/keeper/migrator.gox/mint/keeper/msg_server.gox/nft/go.modx/nft/keeper/class.gox/nft/keeper/genesis.gox/nft/keeper/grpc_query.gox/nft/keeper/keeper.gox/nft/keeper/msg_server.gox/nft/keeper/nft.gox/nft/keeper/nft_batch.gox/params/keeper/grpc_query.gox/params/keeper/keeper.gox/slashing/keeper/genesis.gox/slashing/keeper/grpc_query.gox/slashing/keeper/hooks.gox/slashing/keeper/infractions.gox/slashing/keeper/keeper.gox/slashing/keeper/migrations.gox/slashing/keeper/msg_server.gox/slashing/keeper/params.gox/slashing/keeper/signing_info.gox/slashing/keeper/unjail.gox/staking/keeper/abci.gox/staking/keeper/alias_functions.gox/staking/keeper/delegation.gox/staking/keeper/delegation_receiver.gox/staking/keeper/genesis.gox/staking/keeper/grpc_query.gox/staking/keeper/historical_info.gox/staking/keeper/keeper.gox/staking/keeper/migrations.gox/staking/keeper/msg_server.gox/staking/keeper/params.gox/staking/keeper/pool.gox/staking/keeper/power_reduction.gox/staking/keeper/query_utils.gox/staking/keeper/slash.gox/staking/keeper/unbonding.gox/staking/keeper/val_state_change.gox/staking/keeper/validator.gox/upgrade/go.modx/upgrade/keeper/grpc_query.gox/upgrade/keeper/keeper.gox/upgrade/keeper/migrations.gox/upgrade/keeper/msg_server.go
| golang.org/x/exp v0.0.0-20250106191152-7588d65b2ba8 | ||
| google.golang.org/grpc v1.71.0 | ||
| google.golang.org/protobuf v1.36.5 | ||
| google.golang.org/grpc v1.75.0 |
There was a problem hiding this comment.
❓ Verification inconclusive
Script executed:
#!/bin/bash
set -euo pipefail
echo "OSV query for google.golang.org/grpc@v1.75.0"
curl -sS https://api.osv.dev/v1/query \
-H 'Content-Type: application/json' \
-d '{"package":{"name":"google.golang.org/grpc","ecosystem":"Go"},"version":"1.75.0"}'
echo
echo "Latest published grpc version from Go proxy"
curl -sS https://proxy.golang.org/google.golang.org/grpc/@latestRepository: InjectiveLabs/cosmos-sdk
Repository: InjectiveLabs/cosmos-sdk
Exit code: 0
stdout:
OSV query for google.golang.org/grpc@v1.75.0
{"vulns":[{"id":"GHSA-p77j-4mvh-x3m3","summary":"gRPC-Go has an authorization bypass via missing leading slash in :path","details":"### Impact\n_What kind of vulnerability is it? Who is impacted?_\n\nIt is an **Authorization Bypass** resulting from **Improper Input Validation** of the HTTP/2 `:path` pseudo-header.\n\nThe gRPC-Go server was too lenient in its routing logic, accepting requests where the `:path` omitted the mandatory leading slash (e.g., `Service/Method` instead of `/Service/Method`). While the server successfully routed these requests to the correct handler, authorization interceptors (including the official `grpc/authz` package) evaluated the raw, non-canonical path string. Consequently, \"deny\" rules defined using canonical paths (starting with `/`) failed to match the incoming request, allowing it to bypass the policy if a fallback \"allow\" rule was present.\n\n**Who is impacted?**\nThis affects gRPC-Go servers that meet both of the following criteria:\n1. They use path-based authorization interceptors, such as the official RBAC implementation in `google.golang.org/grpc/authz` or custom interceptors relying on `info.FullMethod` or `grpc.Method(ctx)`.\n2. Their security policy contains specific \"deny\" rules for canonical paths but allows other requests by default (a fallback \"allow\" rule).\n\nThe vulnerability is exploitable by an attacker who can send raw HTTP/2 frames with malformed `:path` headers directly to the gRPC server.\n\n### Patches\n_Has the problem been patched? What versions should users upgrade to?_\n\nYes, the issue has been patched. The fix ensures that any request with a `:path` that does not start with a leading slash is immediately rejected with a `codes.Unimplemented` error, preventing it from reaching authorization interceptors or handlers with a non-canonical path string.\n\nUsers should upgrade to the following versions (or newer):\n* **v1.79.3**\n* The latest **master** branch.\n\nIt is recommended that all users employing path-based authorization (especially `grpc/authz`) upgrade as soon as the patch is available in a tagged release.\n\n### Workarounds\n_Is there a way for users to fix or remediate the vulnerability without upgrading?_\n\nWhile upgrading is the most secure and recommended path, users can mitigate the vulnerability using one of the following methods:\n\n#### 1. Use a Validating Interceptor (Recommended Mitigation)\nAdd an \"outermost\" interceptor to your server that validates the path before any other authorization logic runs:\n\n```go\nfunc pathValidationInterceptor(ctx context.Context, req any, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (any, error) {\n if info.FullMethod == \"\" || info.FullMethod[0] != '/' {\n return nil, status.Errorf(codes.Unimplemented, \"malformed method name\")\n } \n return handler(ctx, req)\n}\n\n// Ensure this is the FIRST interceptor in your chain\ns := grpc.NewServer(\n grpc.ChainUnaryInterceptor(pathValidationInterceptor, authzInterceptor),\n)\n```\n\n#### 2. Infrastructure-Level Normalization\nIf your gRPC server is behind a reverse proxy or load balancer (such as Envoy, NGINX, or an L7 Cloud Load Balancer), ensure it is configured to enforce strict HTTP/2 compliance for pseudo-headers and reject or normalize requests where the `:path` header does not start with a leading slash.\n\n#### 3. Policy Hardening\nSwitch to a \"default deny\" posture in your authorization policies (explicitly listing all allowed paths and denying everything else) to reduce the risk of bypasses via malformed inputs.","aliases":["CVE-2026-33186","GO-2026-4762"],"modified":"2026-03-27T19:11:22.629776Z","published":"2026-03-18T20:10:29Z","related":["CGA-8qwg-3gv7-2fp3"],"database_specific":{"cwe_ids":["CWE-285"],"github_reviewed_at":"2026-03-18T20:10:29Z","github_reviewed":true,"nvd_published_at":"2026-03-20T23:16:45Z","severity":"CRITICAL"},"references":[{"type":"WEB","url":"https://github.com/grpc/grpc-go/security/advisories/GHSA-p77j-4mvh-x3m3"},{"type":"ADVISORY","url":"https://nvd.nist.gov/vuln/detail/CVE-2026-33186"},{"type":"PACKAGE","url":"https://github.com/grpc/grpc-go"}],"affected":[{"package":{"name":"google.golang.org/grpc","ecosystem":"Go","purl":"pkg:golang/google.golang.org/grpc"},"ranges":[{"type":"SEMVER","events":[{"introduced":"0"},{"fixed":"1.79.3"}]}],"database_specific":{"source":"https://github.com/github/advisory-database/blob/main/advisories/github-reviewed/2026/03/GHSA-p77j-4mvh-x3m3/GHSA-p77j-4mvh-x3m3.json"}}],"schema_version":"1.7.5","severity":[{"type":"CVSS_V3","score":"CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:N"}]},{"id":"GO-2026-4762","summary":"Authorization bypass in gRPC-Go via missing leading slash in :path in google.golang.org/grpc","details":"Authorization bypass in gRPC-Go via missing leading slash in :path in google.golang.org/grpc","aliases":["CVE-2026-33186","GHSA-p77j-4mvh-x3m3"],"modified":"2026-03-27T19:00:25.589382Z","published":"2026-03-27T18:39:44Z","database_specific":{"url":"https://pkg.go.dev/vuln/GO-2026-4762","review_status":"REVIEWED"},"references":[{"type":"ADVISORY","url":"https://github.com/grpc/grpc-go/security/advisories/GHSA-p77j-4mvh-x3m3"}],"affected":[{"package":{"name":"google.golang.org/grpc","ecosystem":"Go","purl":"pkg:golang/google.golang.org/grpc"},"ranges":[{"type":"SEMVER","events":[{"introduced":"0"},{"fixed":"1.79.3"}]}],"ecosystem_specific":{"imports":[{"symbols":["Server.Serve","Server.ServeHTTP","Server.handleStream"],"path":"google.golang.org/grpc"}]},"database_specific":{"source":"https://vuln.go.dev/ID/GO-2026-4762.json"}}],"schema_version":"1.7.5"}]}
Latest published grpc version from Go proxy
{"Version":"v1.79.3","Time":"2026-03-17T23:13:23Z","Origin":{"VCS":"git","URL":"https://github.com/grpc/grpc-go","Hash":"dda86dbd9cecb8b35b58c73d507d81d67761205f","Ref":"refs/tags/v1.79.3"}}
Blocker: google.golang.org/grpc v1.75.0 contains a critical authorization bypass vulnerability (GHSA-p77j-4mvh-x3m3).
This dependency must be bumped to v1.79.3 or later before merge. The vulnerability is an authorization bypass resulting from improper validation of the HTTP/2 :path pseudo-header and affects gRPC servers with path-based authorization interceptors.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@store/go.mod` at line 24, Replace the vulnerable dependency entry
"google.golang.org/grpc v1.75.0" with a secure release (at least v1.79.3), e.g.
update the module version to "google.golang.org/grpc v1.79.3"; then run the Go
module update commands (go get google.golang.org/grpc@v1.79.3 and go mod tidy)
to regenerate go.sum and ensure the project builds and tests pass. Ensure the
final go.mod no longer references v1.75.0 anywhere and commit the updated go.mod
and go.sum.
| google.golang.org/grpc v1.71.0 | ||
| github.com/stretchr/testify v1.11.1 | ||
| google.golang.org/genproto/googleapis/api v0.0.0-20250825161204-c5933d9347a5 | ||
| google.golang.org/grpc v1.75.0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and examine its contents around the specified lines
find . -name "go.mod" -path "*/circuit/*" -type fRepository: InjectiveLabs/cosmos-sdk
Length of output: 86
🏁 Script executed:
# Check the specific lines mentioned in the review
if [ -f "x/circuit/go.mod" ]; then
echo "=== Line 22 (grpc) ==="
sed -n '20,24p' "x/circuit/go.mod"
echo ""
echo "=== Line 159 (otel/sdk) ==="
sed -n '157,161p' "x/circuit/go.mod"
fiRepository: InjectiveLabs/cosmos-sdk
Length of output: 566
🌐 Web query:
GHSA-p77j-4mvh-x3m3 google.golang.org/grpc v1.75.0
💡 Result:
GHSA-p77j-4mvh-x3m3 (CVE-2026-33186) is a critical authorization bypass vulnerability (CVSS 9.1) in google.golang.org/grpc, affecting versions before 1.79.3, including v1.75.0 (released around August 2025). It allows attackers to bypass path-based authorization (e.g., grpc/authz deny rules) by sending HTTP/2 requests with malformed :path headers missing the leading slash (e.g., "Service/Method" instead of "/Service/Method"). The server routes these to handlers but interceptors see the non-canonical path, failing to match deny rules with a fallback allow. Patched in v1.79.3 (March 17, 2026), which rejects such requests with codes.Unimplemented. Upgrade to v1.79.3 or later. Mitigations: validating interceptors, strict HTTP/2 proxies, or method-based auth.
Citations:
- 1: GHSA-p77j-4mvh-x3m3
- 2: https://osv.dev/vulnerability/GHSA-p77j-4mvh-x3m3
- 3: GHSA-p77j-4mvh-x3m3
- 4: https://security.snyk.io/vuln/SNYK-GOLANG-GOOGLEGOLANGORGGRPC-15691172
- 5: https://github.com/grpc/grpc-go/releases/tag/v1.79.3
🌐 Web query:
GHSA-9h8m-3fm2-qjrq go.opentelemetry.io/otel v1.38.0
💡 Result:
GHSA-9h8m-3fm2-qjrq is a high-severity vulnerability (CVSS 7.0) in the OpenTelemetry Go SDK, specifically affecting go.opentelemetry.io/otel/sdk/resource on macOS/Darwin systems. It allows arbitrary code execution via local PATH hijacking. The resource detection code in sdk/resource/host_id.go executes the 'ioreg' system command without a secure PATH, enabling an attacker who can modify the PATH environment variable to execute malicious code in the application's context. Affected versions: >=1.21.0 to <=1.39.0 (including v1.38.0). Patched in v1.40.0 via commit d45961b. Aliases: CVE-2026-24051, GO-2026-4394. CWE-426 (Untrusted Search Path). Published February 2, 2026.
Citations:
- 1: GHSA-9h8m-3fm2-qjrq
- 2: https://osv.dev/vulnerability/GHSA-9h8m-3fm2-qjrq
- 3: https://pkg.go.dev/vuln/GO-2026-4394
Upgrade google.golang.org/grpc to v1.79.3+ and go.opentelemetry.io/otel/sdk to v1.40.0+.
Both versions contain known vulnerabilities:
google.golang.org/grpc v1.75.0is vulnerable to GHSA-p77j-4mvh-x3m3 (CVE-2026-33186, CVSS 9.1)—a critical authorization bypass via malformed HTTP/2 :path headers that bypasses authz deny rules. Fixed in v1.79.3.go.opentelemetry.io/otel/sdk v1.38.0is vulnerable to GHSA-9h8m-3fm2-qjrq (CVE-2026-24051, CVSS 7.0)—arbitrary code execution via local PATH hijacking on macOS. Fixed in v1.40.0.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@x/circuit/go.mod` at line 22, Update the vulnerable module versions in
go.mod: change the google.golang.org/grpc dependency to v1.79.3 (or a newer
compatible release) and change go.opentelemetry.io/otel/sdk to v1.40.0 (or
newer); locate these by the module path strings "google.golang.org/grpc" and
"go.opentelemetry.io/otel/sdk" in go.mod, update their version tags, then run go
get ./... (or go get google.golang.org/grpc@v1.79.3
go.opentelemetry.io/otel/sdk@v1.40.0) and go mod tidy to update the lockfile and
resolve transitive changes, then run the test suite/build to verify nothing
broke.
| google.golang.org/genproto/googleapis/api v0.0.0-20250825161204-c5933d9347a5 | ||
| google.golang.org/grpc v1.75.0 | ||
| google.golang.org/protobuf v1.36.8 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if x/evidence/go.mod exists and view lines 25-27
if [ -f "x/evidence/go.mod" ]; then
echo "=== x/evidence/go.mod lines 20-30 ==="
sed -n '20,30p' x/evidence/go.mod
echo ""
echo "=== x/evidence/go.mod lines 150-165 ==="
sed -n '150,165p' x/evidence/go.mod
else
echo "x/evidence/go.mod not found"
git ls-files | grep -i "go.mod"
fiRepository: InjectiveLabs/cosmos-sdk
Length of output: 1351
🏁 Script executed:
# Search for any go.mod files to understand structure
fd -name "go.mod" -type fRepository: InjectiveLabs/cosmos-sdk
Length of output: 298
🌐 Web query:
GHSA-p77j-4mvh-x3m3 google.golang.org/grpc vulnerability
💡 Result:
GHSA-p77j-4mvh-x3m3 is a critical authorization bypass vulnerability (CVE-2026-33186) in google.golang.org/grpc (gRPC-Go), affecting versions prior to 1.79.3. It results from improper input validation of the HTTP/2 :path pseudo-header, where the server accepts requests missing the leading slash (e.g., "Service/Method" instead of "/Service/Method"). This allows bypassing path-based authorization interceptors like those in grpc/authz if they have deny rules for canonical paths but a fallback allow rule. CVSS score: 9.1 (AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:N). CWE-285 (Improper Authorization). Published around March 17-20, 2026. Fixed in v1.79.3 by rejecting invalid :path headers with codes.Unimplemented. Impacts gRPC-Go servers using path-based auth interceptors. Upgrade to v1.79.3 or later.
Citations:
- 1: GHSA-p77j-4mvh-x3m3
- 2: https://osv.dev/vulnerability/GHSA-p77j-4mvh-x3m3
- 3: https://nvd.nist.gov/vuln/detail/CVE-2026-33186
- 4: GHSA-p77j-4mvh-x3m3
🌐 Web query:
GHSA-9h8m-3fm2-qjrq opentelemetry otel sdk vulnerability
💡 Result:
GHSA-9h8m-3fm2-qjrq is a high-severity vulnerability (CVSS 7.0) in the OpenTelemetry Go SDK (go.opentelemetry.io/otel/sdk/resource), affecting versions >=1.21.0 and <=1.39.0. It allows arbitrary code execution via local PATH hijacking on macOS/Darwin systems. The resource detection code in sdk/resource/host_id.go executes the 'ioreg' system command without a secure PATH, enabling an attacker with local PATH modification ability to execute arbitrary code in the application's context. Patched in v1.40.0 (commit d45961b). Also tracked as CVE-2026-24051 and GO-2026-4394. Published February 2, 2026.
Citations:
- 1: GHSA-9h8m-3fm2-qjrq
- 2: https://osv.dev/vulnerability/GHSA-9h8m-3fm2-qjrq
- 3: GHSA-9h8m-3fm2-qjrq
- 4: https://db.gcve.eu/vuln/ghsa-9h8m-3fm2-qjrq
This module includes two critical dependency vulnerabilities requiring immediate patching.
google.golang.org/grpc v1.75.0 is vulnerable to GHSA-p77j-4mvh-x3m3 (authorization bypass, CVE-2026-33186, CVSS 9.1) and requires upgrade to v1.79.3 or later. go.opentelemetry.io/otel/sdk v1.38.0 is vulnerable to GHSA-9h8m-3fm2-qjrq (arbitrary code execution via PATH hijacking, CVE-2026-24051, CVSS 7.0) and requires upgrade to v1.40.0 or later. Upgrade both dependencies before merging.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@x/evidence/go.mod` around lines 25 - 27, The go.mod currently pins vulnerable
versions; update the listed modules to secure releases: bump
google.golang.org/grpc to v1.79.3 or later and go.opentelemetry.io/otel/sdk to
v1.40.0 or later, then run `go get`/`go mod tidy` to update go.sum and ensure no
other indirect deps force older versions; verify build and run tests to confirm
compatibility (look for occurrences of google.golang.org/grpc and
go.opentelemetry.io/otel/sdk to locate the entries to change).
| google.golang.org/protobuf v1.36.5 | ||
| github.com/stretchr/testify v1.11.1 | ||
| google.golang.org/genproto/googleapis/api v0.0.0-20250825161204-c5933d9347a5 | ||
| google.golang.org/grpc v1.75.0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "go.mod" -path "*/feegrant/*" | head -5Repository: InjectiveLabs/cosmos-sdk
Length of output: 87
🏁 Script executed:
cat -n x/feegrant/go.mod | sed -n '20,30p'Repository: InjectiveLabs/cosmos-sdk
Length of output: 478
🏁 Script executed:
cat -n x/feegrant/go.mod | sed -n '170,178p'Repository: InjectiveLabs/cosmos-sdk
Length of output: 676
🌐 Web query:
GHSA-p77j-4mvh-x3m3 google.golang.org/grpc vulnerability
💡 Result:
GHSA-p77j-4mvh-x3m3 is a critical authorization bypass vulnerability (CVE-2026-33186, CVSS 9.1) in google.golang.org/grpc (gRPC-Go) versions prior to 1.79.3. The gRPC-Go server improperly validates the HTTP/2 :path pseudo-header, accepting requests missing the mandatory leading slash (e.g., "Service/Method" instead of "/Service/Method"). While routing succeeds, authorization interceptors (like google.golang.org/grpc/authz) evaluate the raw path without the slash, bypassing path-based "deny" rules if a fallback "allow" exists. Impacts gRPC-Go servers using path-based authorization with explicit denies and default allows. Attackers with HTTP/2 access can send malformed :path headers to bypass restrictions. Fixed in v1.79.3 (released 2026-03-17) by rejecting non-canonical paths with codes.Unimplemented before interceptors (PR #8981). Upgrade to v1.79.3 or later. No known workarounds besides avoiding path-based auth or disabling HTTP/2.
Citations:
- 1: GHSA-p77j-4mvh-x3m3
- 2: https://osv.dev/vulnerability/GHSA-p77j-4mvh-x3m3
- 3: https://nvd.nist.gov/vuln/detail/CVE-2026-33186
- 4: GHSA-p77j-4mvh-x3m3
- 5: https://github.com/grpc/grpc-go/releases/tag/v1.79.3
🌐 Web query:
GHSA-9h8m-3fm2-qjrq go.opentelemetry.io/otel/sdk vulnerability
💡 Result:
GHSA-9h8m-3fm2-qjrq is a high-severity vulnerability (CVSS 7.0) in the go.opentelemetry.io/otel/sdk package, also known as CVE-2026-24051 and GO-2026-4394. It affects versions from v1.21.0 (or v1.20.0 per some sources) up to but not including v1.40.0, specifically on macOS/Darwin systems. The vulnerability is due to untrusted search path (CWE-426) in the resource detection code at sdk/resource/host_id.go. It executes the 'ioreg' system command without a fully qualified path, allowing an attacker who can modify the PATH environment variable to hijack the command and achieve arbitrary code execution (ACE) in the application's context. Attack requires local access with low privileges but high attack complexity (AV:L/AC:H/PR:L). The issue was published on 2026-02-02 and patched in commit d45961b, released in v1.40.0 on the same day. Users should upgrade to v1.40.0 or later. As of 2026-03-30, later versions like v1.42.0 are available. Workarounds: Do not use resource detection on untrusted macOS environments or ensure PATH cannot be modified by untrusted parties.
Citations:
- 1: GHSA-9h8m-3fm2-qjrq
- 2: https://osv.dev/vulnerability/GHSA-9h8m-3fm2-qjrq
- 3: https://db.gcve.eu/vuln/ghsa-9h8m-3fm2-qjrq
- 4: GHSA-9h8m-3fm2-qjrq
- 5: https://pkg.go.dev/vuln/GO-2026-4394
- 6: https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md
- 7: https://github.com/open-telemetry/opentelemetry-go/releases
- 8: https://vulert.com/vuln-db/CVE-2026-24051
- 9: https://advisories.gitlab.com/pkg/golang/go.opentelemetry.io/otel/sdk/CVE-2026-24051/
Upgrade blocked dependencies to patch known vulnerabilities.
google.golang.org/grpc v1.75.0 contains a critical authorization bypass vulnerability (GHSA-p77j-4mvh-x3m3, CVE-2026-33186, CVSS 9.1) that allows path-based authorization bypass in gRPC servers. Upgrade to v1.79.3 or later.
go.opentelemetry.io/otel/sdk v1.38.0 contains a high-severity code execution vulnerability (GHSA-9h8m-3fm2-qjrq, CVE-2026-24051, CVSS 7.0) on macOS due to unsafe command execution. Upgrade to v1.40.0 or later.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@x/feegrant/go.mod` at line 25, Update the vulnerable module versions in
go.mod: bump google.golang.org/grpc to v1.79.3 (or later) and
go.opentelemetry.io/otel/sdk to v1.40.0 (or later); then run go get
google.golang.org/grpc@v1.79.3 and go get go.opentelemetry.io/otel/sdk@v1.40.0
(or use your chosen newer versions) and run go mod tidy to update the lockfile
and ensure dependent packages are resolved; target the module names
google.golang.org/grpc and go.opentelemetry.io/otel/sdk in the file and verify
tests/build succeed.
| google.golang.org/genproto/googleapis/api v0.0.0-20250825161204-c5933d9347a5 | ||
| google.golang.org/grpc v1.75.0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n x/nft/go.mod | head -30Repository: InjectiveLabs/cosmos-sdk
Length of output: 1294
🏁 Script executed:
cat -n x/nft/go.mod | sed -n '154,160p'Repository: InjectiveLabs/cosmos-sdk
Length of output: 565
🏁 Script executed:
wc -l x/nft/go.modRepository: InjectiveLabs/cosmos-sdk
Length of output: 84
🌐 Web query:
GHSA-p77j-4mvh-x3m3 google.golang.org/grpc
💡 Result:
GHSA-p77j-4mvh-x3m3 is a critical authorization bypass vulnerability (CVE-2026-33186) in google.golang.org/grpc (gRPC-Go). Summary: The gRPC-Go server improperly validates the HTTP/2 :path pseudo-header, accepting requests without a leading slash (e.g., "Service/Method" instead of "/Service/Method"). This allows bypassing path-based authorization interceptors like those in grpc/authz, as they match on the raw non-canonical path. Affected: Versions < 1.79.3 of google.golang.org/grpc. Fixed: Upgrade to v1.79.3 or later. Severity: CVSS 3.1 9.1 (Critical) - AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:N. CWE-285 (Improper Authorization). Impact: Affects servers using path-based auth interceptors with deny rules for canonical paths and fallback allow. Exploitable remotely by sending malformed HTTP/2 frames. Published: March 17-18, 2026. As of 2026-03-30, patched and recent. Mitigation: Upgrade immediately. Optionally, add custom unary interceptor to reject non-/ paths with codes.Unimplemented.
Citations:
- 1: GHSA-p77j-4mvh-x3m3
- 2: https://osv.dev/vulnerability/GHSA-p77j-4mvh-x3m3
- 3: GHSA-p77j-4mvh-x3m3
- 4: https://nvd.nist.gov/vuln/detail/CVE-2026-33186
🌐 Web query:
GHSA-9h8m-3fm2-qjrq go.opentelemetry.io/otel
💡 Result:
GHSA-9h8m-3fm2-qjrq is a high-severity security vulnerability (CVSS 7.0) in the OpenTelemetry Go SDK package go.opentelemetry.io/otel/sdk, affecting versions >=1.21.0 and <=1.39.0. It allows arbitrary code execution via local PATH hijacking on macOS/Darwin systems. The resource detection code in sdk/resource/host_id.go executes the 'ioreg' system command without a secure PATH, enabling an attacker with local access to modify PATH to achieve ACE in the application's context. Patched in v1.40.0 (commit d45961b). Aliases: CVE-2026-24051, GO-2026-4394. Published Feb 2, 2026.
Citations:
- 1: GHSA-9h8m-3fm2-qjrq
- 2: https://osv.dev/vulnerability/GHSA-9h8m-3fm2-qjrq
- 3: https://db.gcve.eu/vuln/ghsa-9h8m-3fm2-qjrq
- 4: GHSA-9h8m-3fm2-qjrq
- 5: https://pkg.go.dev/vuln/GO-2026-4394
Resolve critical security vulnerabilities in dependencies.
This module pins versions with known high-severity and critical vulnerabilities that are flagged in dependency review:
google.golang.org/grpc v1.75.0(GHSA-p77j-4mvh-x3m3): Critical authorization bypass—affects versions < 1.79.3. Upgrade to v1.79.3 or later.go.opentelemetry.io/otel/sdk v1.38.0(GHSA-9h8m-3fm2-qjrq): High-severity arbitrary code execution via local PATH hijacking—affects versions 1.21.0 to 1.39.0. Upgrade to v1.40.0 or later.
Both packages require bumping before merge (lines 24 and 154–160).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@x/nft/go.mod` around lines 23 - 24, Update the vulnerable dependency versions
in go.mod: bump google.golang.org/grpc from v1.75.0 to v1.79.3 (or later) and
bump go.opentelemetry.io/otel/sdk from v1.38.0 to v1.40.0 (or later); adjust any
other referenced otel modules in the same section (lines around the otel/sdk
block) to compatible versions, then run go get ./... or go get
<module>@<version> and go mod tidy and run tests/build to ensure no breakage.
Ensure the module names exactly match google.golang.org/grpc and
go.opentelemetry.io/otel/sdk so the dependency review picks up the fixed
versions.
| func (k *Keeper) Meter(ctx context.Context) metrics.Meter { | ||
| if k.meter == nil { | ||
| k.meter = sdk.UnwrapSDKContext(ctx).Meter().SubMeter(proposal.ModuleName, metrics.Tag("svc", proposal.ModuleName)) | ||
| } | ||
|
|
||
| return k.meter | ||
| } |
There was a problem hiding this comment.
Data race on lazy meter initialization.
The Meter() method performs unsynchronized read-write on k.meter. Concurrent gRPC requests can race on the nil check and assignment, leading to undefined behavior.
Since keepers are typically long-lived and shared across goroutines handling concurrent requests, this needs synchronization.
🔒 Proposed fix using sync.Once
import (
"context"
+ "sync"
"cosmossdk.io/log"
storetypes "cosmossdk.io/store/types"
metrics "github.com/InjectiveLabs/metrics/v2"
...
type Keeper struct {
+ meterOnce sync.Once
meter metrics.Meter
cdc codec.BinaryCodec
...
func (k *Keeper) Meter(ctx context.Context) metrics.Meter {
- if k.meter == nil {
- k.meter = sdk.UnwrapSDKContext(ctx).Meter().SubMeter(proposal.ModuleName, metrics.Tag("svc", proposal.ModuleName))
- }
-
+ k.meterOnce.Do(func() {
+ k.meter = sdk.UnwrapSDKContext(ctx).Meter().SubMeter(proposal.ModuleName, metrics.Tag("svc", proposal.ModuleName))
+ })
return k.meter
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@x/params/keeper/keeper.go` around lines 81 - 87, The Meter method has a data
race: it reads and writes k.meter without synchronization (nil check +
assignment) causing races for concurrent gRPC requests; fix by making lazy
initialization thread-safe—add a sync.Once (or a mutex) field on Keeper (e.g.,
meterOnce or meterMu) and use it in Meter to ensure the SubMeter creation and
assignment to k.meter happens exactly once, while subsequent calls simply return
k.meter; reference the Meter method, k.meter, and Keeper type when applying this
change.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| case comet.LightClientAttack, comet.DuplicateVote: | ||
| evidence := types.FromABCIEvidence(evidences.Get(i), k.stakingKeeper.ConsensusAddressCodec()) | ||
| err := k.handleEquivocationEvidence(ctx, evidence) | ||
| err := k.handleEquivocationEvidence(sdkCtx, evidence) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods
|
|
||
| defer telemetry.ModuleMeasureSince(types.ModuleName, telemetry.Now(), telemetry.MetricKeyBeginBlocker) | ||
| return k.TrackHistoricalInfo(ctx) | ||
| return k.TrackHistoricalInfo(sdkCtx) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods
|
|
||
| defer telemetry.ModuleMeasureSince(types.ModuleName, telemetry.Now(), telemetry.MetricKeyEndBlocker) | ||
| return k.BlockValidatorUpdates(ctx) | ||
| return k.BlockValidatorUpdates(sdkCtx) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
2 participants
Introduces injective metrics and traces code paths of interest
MERGE ONLY after InjectiveLabs/cometbft#10 has been merged and tagged and this PR's
go.modis updatedDescription
Closes: #XXXX
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.mdmake lintandmake testReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!in the type prefix if API or client breaking changeSummary by CodeRabbit
New Features
Chores