Conversation
Signed-off-by: Artur Troian <troian@users.noreply.github.com>
WalkthroughThis pull request represents a major version bump from v1 to v2, introducing CosmWasm smart contract support (wasm module), a new oracle module, an Akash wasm deployment module, a price-oracle contract, upgraded build infrastructure with wasm VM library handling, and corresponding test/upgrade infrastructure updates. Import paths are systematically updated across the codebase, legacy upgrade code is removed, and a new v2.0.0 upgrade path is implemented. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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: 10
Note
Due to the large number of review comments, Critical, Major 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 (11)
x/market/client/rest/rest.go (1)
13-138:⚠️ Potential issue | 🟠 MajorRemove the REST endpoint file entirely—it is completely unused dead code.
Verification confirms that
x/market/client/rest/rest.gois not imported or referenced anywhere in the codebase. TheRegisterRoutesfunction and all handler implementations are never called. The market module has migrated to gRPC endpoints viaRegisterGRPCGatewayRoutesand no longer needs REST API support. Files with only commented-out code and zero functionality should be deleted rather than maintained.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/market/client/rest/rest.go` around lines 13 - 138, This file contains dead REST endpoints (RegisterRoutes, listOrdersHandler, listBidsHandler, listLeasesHandler, getOrderHandler, getBidHandler, getLeaseHandler) with only commented-out code and is not referenced anywhere; delete x/market/client/rest/rest.go entirely and remove any leftover references to these symbols if present so the codebase only uses the gRPC gateway (RegisterGRPCGatewayRoutes).x/take/module.go (1)
109-112:⚠️ Potential issue | 🟡 MinorComment references "provider" instead of "take".
The comment on line 109 says "provider module name" but this is the take module's Name function.
Proposed fix
-// Name returns the provider module name +// Name returns the take module name func (AppModule) Name() string { return types.ModuleName }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/take/module.go` around lines 109 - 112, The comment above AppModule.Name wrongly says "provider module name"; update the doc comment to refer to the take module instead. Locate the AppModule.Name method and change the comment from something like "Name returns the provider module name" to "Name returns the take module name" (or similar accurate wording) so the comment matches the function (AppModule.Name) and the module identity (types.ModuleName).x/cert/module.go (2)
86-91:⚠️ Potential issue | 🟡 MinorInconsistent error message references "provider" instead of "cert".
The panic message on line 89 still references "provider grpc routes" but this is the cert module.
Proposed fix
func (AppModuleBasic) RegisterGRPCGatewayRoutes(clientCtx client.Context, mux *runtime.ServeMux) { err := types.RegisterQueryHandlerClient(context.Background(), mux, types.NewQueryClient(clientCtx)) if err != nil { - panic(fmt.Sprintf("couldn't register provider grpc routes: %s", err.Error())) + panic(fmt.Sprintf("couldn't register cert grpc routes: %s", err.Error())) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/cert/module.go` around lines 86 - 91, In AppModuleBasic.RegisterGRPCGatewayRoutes, the panic message wrongly references "provider grpc routes"; update the error message to correctly reference the cert module (e.g., "couldn't register cert grpc routes") when types.RegisterQueryHandlerClient returns an error, so the panic includes the correct module name and the error details; ensure you keep err.Error() or use fmt.Errorf to include err when calling panic.
152-154:⚠️ Potential issue | 🟡 MinorComment references "audit" instead of "cert".
The comment on line 152 says "audit module" but this is the cert module's ExportGenesis function.
Proposed fix
-// ExportGenesis returns the exported genesis state as raw bytes for the audit -// module. +// ExportGenesis returns the exported genesis state as raw bytes for the cert +// module. func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.RawMessage {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/cert/module.go` around lines 152 - 154, The function comment above AppModule.ExportGenesis incorrectly refers to the "audit module"; update the comment text to reference the correct module name ("cert module") and/or remove the wrong module name so the docstring accurately describes AppModule.ExportGenesis for the cert module.app/upgrades.go (1)
37-37:⚠️ Potential issue | 🟡 MinorTypo: "unitialize" should be "initialize".
Minor typo in the error message.
📝 Proposed fix
- return fmt.Errorf("unable to unitialize upgrade `%s`: %w", name, err) + return fmt.Errorf("unable to initialize upgrade `%s`: %w", name, err)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/upgrades.go` at line 37, The error message in the return fmt.Errorf call contains a typo: change the string "unable to unitialize upgrade `%s`: %w" to "unable to initialize upgrade `%s`: %w" in the code that returns fmt.Errorf (the call using name and err) so the error reads correctly; update the literal in upgrades.go where that fmt.Errorf is constructed.x/market/keeper/keeper.go (1)
559-565:⚠️ Potential issue | 🟡 MinorDuplicate
deferfor iterator close.There are two identical
deferstatements to close the same iterator. The second one (lines 563-565) is redundant and should be removed.🧹 Proposed fix
func (k Keeper) WithBids(ctx sdk.Context, fn func(types.Bid) bool) { store := ctx.KVStore(k.skey) iter := storetypes.KVStorePrefixIterator(store, keys.BidPrefix) defer func() { _ = iter.Close() }() - defer func() { - _ = iter.Close() - }() - for ; iter.Valid(); iter.Next() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/market/keeper/keeper.go` around lines 559 - 565, Remove the redundant duplicate defer that calls iter.Close(): there are two identical defer blocks both calling _ = iter.Close(), so delete the second defer (the later one) and keep a single defer that closes iter to avoid double-declaring the same cleanup; locate the duplicate around the iterator handling in keeper.go (the defer wrapping _ = iter.Close()) and remove the extra block.x/escrow/handler/handler.go (1)
12-13:⚠️ Potential issue | 🟡 MinorIncorrect doc comment: refers to "deployment" instead of "escrow".
The comment says
"deployment" type messagesbut this handler is for escrow messages.📝 Proposed fix
-// NewHandler returns a handler for "deployment" type messages +// NewHandler returns a handler for "escrow" type messages func NewHandler(keeper keeper.Keeper, authzKeeper AuthzKeeper, bkeeper BankKeeper) baseapp.MsgServiceHandler {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/escrow/handler/handler.go` around lines 12 - 13, Update the doc comment for NewHandler to accurately describe the handler's purpose for escrow messages (not "deployment"); locate the NewHandler function signature (NewHandler(keeper keeper.Keeper, authzKeeper AuthzKeeper, bkeeper BankKeeper) baseapp.MsgServiceHandler) and change the comment to state it returns a handler for "escrow" type messages (or more precise module-specific wording) so the documentation matches the implementation.x/audit/module.go (2)
94-100:⚠️ Potential issue | 🟡 MinorIncorrect module name in doc comment.
The doc comment on line 94 says "provider module" but this method is for the audit module. The panic message on line 98 correctly says "audit", so only the doc comment needs updating.
Proposed fix
-// RegisterGRPCRoutes registers the gRPC Gateway routes for the provider module. +// RegisterGRPCRoutes registers the gRPC Gateway routes for the audit module. func (AppModuleBasic) RegisterGRPCRoutes(clientCtx client.Context, mux *runtime.ServeMux) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/audit/module.go` around lines 94 - 100, Update the doc comment for the RegisterGRPCRoutes method on AppModuleBasic to correctly refer to the audit module (not the provider module); locate the RegisterGRPCRoutes function and replace "provider module" in its top comment with "audit module" so the comment matches the implementation and existing panic message.
86-92:⚠️ Potential issue | 🟡 MinorIncorrect module name in panic message.
The panic message on line 90 says "couldn't register provider grpc routes" but this is the audit module. This appears to be a copy-paste error.
Proposed fix
func (AppModuleBasic) RegisterGRPCGatewayRoutes(clientCtx client.Context, mux *runtime.ServeMux) { err := types.RegisterQueryHandlerClient(context.Background(), mux, types.NewQueryClient(clientCtx)) if err != nil { - panic(fmt.Sprintf("couldn't register provider grpc routes: %s", err.Error())) + panic(fmt.Sprintf("couldn't register audit grpc routes: %s", err.Error())) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/audit/module.go` around lines 86 - 92, The panic in AppModuleBasic.RegisterGRPCGatewayRoutes uses the wrong module name ("provider") — update the panic message to reference the audit module; specifically, in the RegisterGRPCGatewayRoutes function where types.RegisterQueryHandlerClient is called and the error is handled, change the fmt.Sprintf message from "couldn't register provider grpc routes: %s" to something like "couldn't register audit grpc routes: %s" so it correctly identifies the audit module.x/escrow/module.go (1)
86-91:⚠️ Potential issue | 🟡 MinorInconsistent error message: still references "provider" instead of "escrow".
The comment on line 86 was updated to reference the "escrow module", but the error message on line 90 still says "provider grpc routes". This should be updated for consistency.
Proposed fix
// RegisterGRPCGatewayRoutes registers the gRPC Gateway routes for the escrow module. func (AppModuleBasic) RegisterGRPCGatewayRoutes(clientCtx client.Context, mux *runtime.ServeMux) { err := v1.RegisterQueryHandlerClient(context.Background(), mux, v1.NewQueryClient(clientCtx)) if err != nil { - panic(fmt.Sprintf("couldn't register provider grpc routes: %s", err.Error())) + panic(fmt.Sprintf("couldn't register escrow grpc routes: %s", err.Error())) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/escrow/module.go` around lines 86 - 91, The panic message in AppModuleBasic.RegisterGRPCGatewayRoutes still says "couldn't register provider grpc routes" which is inconsistent with the escrow module; update the error string to reference "escrow" (e.g., "couldn't register escrow grpc routes" or "couldn't register escrow module grpc routes") so the message matches the function and comment in RegisterGRPCGatewayRoutes.app/modules.go (1)
228-234:⚠️ Potential issue | 🟡 MinorDuplicate
authzmodule.NewAppModuleregistration in simulation modules.The
authzmodule.NewAppModuleappears twice inappSimModules(lines 228-234 and 248-254). This will register the same module twice, which could cause issues during simulation.🛠️ Proposed fix
Remove the duplicate registration at lines 248-254:
app.interfaceRegistry, ), - authzmodule.NewAppModule( - app.cdc, - app.Keepers.Cosmos.Authz, - app.Keepers.Cosmos.Acct, - app.Keepers.Cosmos.Bank, - app.interfaceRegistry, - ), gov.NewAppModule(Also applies to: 248-254
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/modules.go` around lines 228 - 234, There is a duplicate registration of authzmodule.NewAppModule inside the appSimModules list; remove the redundant call so the simulation modules include only one authzmodule.NewAppModule. Locate the two occurrences of authzmodule.NewAppModule that pass app.cdc, app.Keepers.Cosmos.Authz, app.Keepers.Cosmos.Acct, app.Keepers.Cosmos.Bank, app.interfaceRegistry and delete the second/duplicate entry (keep the first), ensuring appSimModules no longer registers the same module twice.
🟡 Minor comments (12)
x/wasm/handler/server.go-20-38 (1)
20-38:⚠️ Potential issue | 🟡 MinorFix the constructor comment; the nil-guard pattern is inconsistent across handler implementations.
The comment incorrectly references "staking" instead of "wasm". While a nil guard for
reqis defensible, none of theUpdateParamsimplementations across the codebase (x/wasm, x/take, x/oracle, x/market, x/deployment) include one in handler/server.go files. Nil/empty validation is consistently applied only in query handlers (keeper/grpc_query.go), suggesting a deliberate architectural separation. This may warrant module-wide review if the pattern should change.✅ Minimal fix (comment only)
-// NewMsgServerImpl returns an implementation of the akash staking MsgServer interface +// NewMsgServerImpl returns an implementation of the akash wasm MsgServer interface // for the provided Keeper.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/wasm/handler/server.go` around lines 20 - 38, Update the top-of-file constructor comment for NewMsgServerImpl to reference the wasm module (replace "staking" with "wasm") so it correctly describes the returned MsgServer; do not add or remove nil-request guards in UpdateParams (leave the existing logic in msgServer.UpdateParams as-is) since nil/empty validation is handled elsewhere.x/oracle/handler/server.go-20-38 (1)
20-38:⚠️ Potential issue | 🟡 MinorFix the constructor comment and add a nil request guard.
The comment incorrectly references "staking" instead of "oracle", and
UpdateParamscan panic ifreqis nil.Suggested fix
import ( "context" sdk "github.com/cosmos/cosmos-sdk/types" govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" types "pkg.akt.dev/go/node/oracle/v1" "pkg.akt.dev/node/v2/x/oracle/keeper" ) -// NewMsgServerImpl returns an implementation of the akash staking MsgServer interface +// NewMsgServerImpl returns an implementation of the akash oracle MsgServer interface // for the provided Keeper. func NewMsgServerImpl(k keeper.Keeper) types.MsgServer { return &msgServer{ keeper: k, } } func (ms msgServer) UpdateParams(goCtx context.Context, req *types.MsgUpdateParams) (*types.MsgUpdateParamsResponse, error) { + if req == nil { + return nil, status.Error(codes.InvalidArgument, "invalid request") + } if ms.keeper.GetAuthority() != req.Authority { return nil, govtypes.ErrInvalidSigner.Wrapf("invalid authority; expected %s, got %s", ms.keeper.GetAuthority(), req.Authority) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/oracle/handler/server.go` around lines 20 - 38, Update the NewMsgServerImpl comment to reference the oracle MsgServer (not staking) and add a nil-request guard at the start of UpdateParams: check if req == nil and return an appropriate error (e.g., sdkerrors.Wrap or fmt.Errorf) before accessing req.Authority or req.Params; keep the authority check using ms.keeper.GetAuthority() and existing SetParams call on msgServer/UpdateParams/MsgUpdateParams to avoid panics when callers pass nil.x/oracle/keeper/keeper.go-23-83 (1)
23-83:⚠️ Potential issue | 🟡 MinorFix stale comments referencing other modules.
The comments mention “deployment store,” “x/mint,” and “x/take” in the oracle keeper. Please align them with the oracle module to avoid confusion.✏️ Example cleanup
-// Keeper of the deployment store +// Keeper of the oracle store ... -// NewKeeper creates and returns an instance of take keeper +// NewKeeper creates and returns an instance of oracle keeper ... -// GetAuthority returns the x/mint module's authority. +// GetAuthority returns the oracle module's authority. ... -// SetParams sets the x/take module parameters. +// SetParams sets the oracle module parameters. ... -// GetParams returns the current x/take module parameters. +// GetParams returns the current oracle module parameters.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/oracle/keeper/keeper.go` around lines 23 - 83, Update stale comments and docstrings in the keeper code to reference the oracle module instead of other modules: change the top-line comment for the keeper struct from "Keeper of the deployment store" to something like "Keeper of the oracle store", update the NewKeeper comment to "creates and returns an instance of oracle keeper", change the GetAuthority comment to mention the oracle module (or the correct module account) rather than "x/mint", and rename any "x/take" mentions in SetParams/GetParams comments to "x/oracle" (or simply "oracle module parameters"); ensure comments adjacent to functions Codec, StoreKey, NewQuerier, SetParams, GetParams and the keeper struct are consistent with the oracle module terminology._run/.envrc-3-9 (1)
3-9:⚠️ Potential issue | 🟡 MinorError messages don't reset terminal color.
The error messages set the terminal color to red but don't reset it afterward, which may cause subsequent terminal output to remain red.
🔧 Proposed fix to reset terminal color
if ! has grpcurl ; then - echo -e "\033[31mgrpcurl is not installed"; exit 1 + echo -e "\033[31mgrpcurl is not installed\033[0m"; exit 1 fi if ! has tqdm ; then - echo -e "\033[31mtqdm is not installed. https://github.com/tqdm/tqdm"; exit 1 + echo -e "\033[31mtqdm is not installed. https://github.com/tqdm/tqdm\033[0m"; exit 1 fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@_run/.envrc` around lines 3 - 9, The error messages that set red color via "\033[31m" in the checks for "has grpcurl" and "has tqdm" don't reset the terminal color; update both echo statements used in the blocks that test "has grpcurl" and "has tqdm" so they append a reset sequence (e.g., "\033[0m" or use "tput sgr0") after the message to restore the terminal color before exiting, keeping the same wording and exit behavior.x/wasm/genesis.go-20-21 (1)
20-21:⚠️ Potential issue | 🟡 MinorFix comment: references "deployment" instead of "wasm".
The comment incorrectly references the "deployment module" - this should be "wasm module".
Proposed fix
-// DefaultGenesisState returns default genesis state as raw bytes for the deployment -// module. +// DefaultGenesisState returns default genesis state as raw bytes for the wasm +// module.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/wasm/genesis.go` around lines 20 - 21, Update the top-of-file comment for DefaultGenesisState to reference the wasm module instead of the deployment module: locate the comment above the DefaultGenesisState function in genesis.go and replace "deployment module" with "wasm module" so the docstring correctly reads that it returns the default genesis state for the wasm module.x/wasm/genesis.go-44-45 (1)
44-45:⚠️ Potential issue | 🟡 MinorFix comment: references "deployment" instead of "wasm".
The comment incorrectly references the "deployment module" - this should be "wasm module".
Proposed fix
-// ExportGenesis returns genesis state for the deployment module +// ExportGenesis returns genesis state for the wasm module🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/wasm/genesis.go` around lines 44 - 45, The top-level comment above ExportGenesis incorrectly says "deployment module" instead of "wasm module"; update the comment for the ExportGenesis function to reference the wasm module (e.g., change "deployment module" to "wasm module") so the documentation matches the package purpose and the function ExportGenesis.x/wasm/simulation/genesis.go-9-16 (1)
9-16:⚠️ Potential issue | 🟡 MinorFix copy-paste artifacts: variable name and comment reference wrong modules.
The variable
takeGenesisand the comment "for supply" appear to be copied from another module. These should reference the wasm module for clarity.Proposed fix
-// RandomizedGenState generates a random GenesisState for supply +// RandomizedGenState generates a random GenesisState for wasm func RandomizedGenState(simState *module.SimulationState) { - takeGenesis := &types.GenesisState{ + wasmGenesis := &types.GenesisState{ Params: types.DefaultParams(), } - simState.GenState[types.ModuleName] = simState.Cdc.MustMarshalJSON(takeGenesis) + simState.GenState[types.ModuleName] = simState.Cdc.MustMarshalJSON(wasmGenesis) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/wasm/simulation/genesis.go` around lines 9 - 16, Update the copy-paste artifacts in RandomizedGenState: change the top comment to reference the wasm module (e.g., "RandomizedGenState generates a random GenesisState for wasm") and rename the local variable takeGenesis to a clear wasm-specific name such as wasmGenesis or genesisState; ensure the variable still holds types.GenesisState with Params: types.DefaultParams() and is marshaled into simState.GenState[types.ModuleName] as before._run/common-commands.mk-29-47 (1)
29-47:⚠️ Potential issue | 🟡 MinorExit status check captures
readresult, not the pipeline result.On line 37,
$$?captures the exit status of thereadcommand, not thecurl | jqpipeline. Additionally, ifjqreturns"null"or an empty string (e.g., when the node isn't ready), the-lt 1comparison may produce a shell error rather than evaluating to false.🛠️ Proposed fix with robust validation
akash-node-ready: SHELL=$(BASH_PATH) akash-node-ready: @( \ max_retry=15; \ counter=0; \ while [[ $$counter -lt $$max_retry ]]; do \ - read block < <(curl -s $(AKASH_NODE)/status | jq -r '.result.sync_info.latest_block_height' 2> /dev/null); \ - if [[ $$? -ne 0 || $$block -lt 1 ]]; then \ + block=$$(curl -s $(AKASH_NODE)/status 2>/dev/null | jq -r '.result.sync_info.latest_block_height' 2>/dev/null); \ + if [[ -z "$$block" || "$$block" == "null" || ! "$$block" =~ ^[0-9]+$$ || "$$block" -lt 1 ]]; then \ echo "unable to get node status. sleep for 1s"; \ ((counter++)); \ sleep 1; \ else \ echo "latest block height: $${block}"; \ exit 0; \ fi \ done; \ exit 1 \ )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@_run/common-commands.mk` around lines 29 - 47, The akash-node-ready target's check uses $$? immediately after read, which only reflects read's exit status and not the curl|jq pipeline, and comparing $$block with -lt 1 can fail when jq returns empty/null; change the logic in the akash-node-ready target to capture the pipeline output into a variable (e.g., assign block with command substitution from curl ... | jq -r ...), check the pipeline exit status (use set -o pipefail or inspect curl and jq exit codes) and then validate that the block value is a non-empty integer (e.g., regex or grep '^[0-9]\+$') before doing a numeric comparison against 1; update the loop to treat non-numeric/empty values as failure and retry rather than causing a shell error.go.mod-3-3 (1)
3-3:⚠️ Potential issue | 🟡 MinorUpdate Go version to latest available.
Go 1.25.4 is valid but outdated as of February 2026. Consider upgrading to Go 1.25.7 (latest 1.25 patch) or Go 1.26.0 (latest stable).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` at line 3, Update the Go version declaration "go 1.25.4" in the go.mod file to a current release (e.g., "go 1.25.7" or "go 1.26.0"); after changing the "go" directive, run go mod tidy and all relevant tests/builds to ensure module resolution and compatibilities are OK and address any minor breakages reported by the toolchain.x/wasm/keeper/keeper.go-27-95 (1)
27-95:⚠️ Potential issue | 🟡 MinorFix copy‑paste doc comments (module names are incorrect).
The comments currently reference deployment/take/mint, but this is the wasm keeper.✍️ Suggested comment fixes
-// Keeper of the deployment store +// Keeper of the wasm store ... -// NewKeeper creates and returns an instance of take keeper +// NewKeeper creates and returns an instance of the wasm keeper ... -// GetAuthority returns the x/mint module's authority. +// GetAuthority returns the module authority (x/gov module account). ... -// SetParams sets the x/take module parameters. +// SetParams sets the wasm module parameters. ... -// GetParams returns the current x/take module parameters. +// GetParams returns the current wasm module parameters.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/wasm/keeper/keeper.go` around lines 27 - 95, Update the incorrect copy-paste comments to reference the wasm module and its responsibilities: change the file-level comment above type keeper to say "Keeper of the wasm module store" (or similar), update the NewKeeper comment to "NewKeeper creates and returns an instance of the wasm keeper", change the Codec comment to "Codec returns keeper codec for wasm", change StoreKey/NewQuerier comments if present to reference wasm, and update GetAuthority/SetParams/GetParams comments to mention the wasm module (e.g., "GetAuthority returns the wasm module authority", "SetParams sets the wasm module parameters", "GetParams returns the current wasm module parameters"). Ensure each comment sits immediately above the matching symbols: keeper, NewKeeper, Codec, StoreKey, NewQuerier, GetAuthority, SetParams, GetParams.x/wasm/module.go-110-181 (1)
110-181:⚠️ Potential issue | 🟡 MinorFix copy‑paste comments to reference the wasm module.
These comments currently mention provider/staking/take, which is misleading.✍️ Suggested comment fixes
-// Name returns the provider module name +// Name returns the wasm module name ... -// GenerateGenesisState creates a randomized GenState of the staking module. +// GenerateGenesisState creates a randomized GenState of the wasm module. ... -// RegisterStoreDecoder registers a decoder for take module's types. +// RegisterStoreDecoder registers a decoder for wasm module's types. ... -// WeightedOperations doesn't return any take module operation. +// WeightedOperations doesn't return any wasm module operation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/wasm/module.go` around lines 110 - 181, The file contains several incorrect copy‑paste comments that reference other modules (provider/staking/take) around AppModule methods; update the comments to accurately describe the wasm module behavior for functions like Name, IsOnePerModuleType, IsAppModule, QuerierRoute, RegisterServices, BeginBlock, EndBlock, InitGenesis, ExportGenesis, ConsensusVersion, GenerateGenesisState, ProposalMsgs, RegisterStoreDecoder and WeightedOperations so they mention the wasm module (e.g., "wasm module" or "this module") and correct descriptions (e.g., "BeginBlock performs no-op", "EndBlock returns no validator updates", "GenerateGenesisState creates randomized genesis for wasm module", etc.). Ensure each comment is concise and consistent with the corresponding method semantics and remove references to provider/staking/take.x/oracle/module.go-110-181 (1)
110-181:⚠️ Potential issue | 🟡 MinorFix copy‑paste comments to reference the oracle module.
These comments currently mention provider/staking/take, which is misleading.✍️ Suggested comment fixes
-// Name returns the provider module name +// Name returns the oracle module name ... -// GenerateGenesisState creates a randomized GenState of the staking module. +// GenerateGenesisState creates a randomized GenState of the oracle module. ... -// RegisterStoreDecoder registers a decoder for take module's types. +// RegisterStoreDecoder registers a decoder for oracle module's types. ... -// WeightedOperations doesn't return any take module operation. +// WeightedOperations doesn't return any oracle module operation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/oracle/module.go` around lines 110 - 181, Update misleading copy‑paste comments to reference the oracle module: change the comment above Name() from "provider module name" to "oracle module name", ensure comments for GenerateGenesisState, RegisterStoreDecoder, and WeightedOperations no longer mention "staking" or "take" (e.g., use "oracle module" or "oracle module's types/operations"), and scan the other comments (IsOnePerModuleType, IsAppModule, QuerierRoute, RegisterServices, BeginBlock, EndBlock, InitGenesis, ExportGenesis, ConsensusVersion, ProposalMsgs) to make sure they consistently mention the oracle module where appropriate and correct any remaining copy‑paste wording.
🧹 Nitpick comments (15)
wasmvm.go (1)
3-3: Avoid baking repo-local rpath into production binaries.
-Wl,-rpath,${SRCDIR}/.cache/libhardcodes a build-cache location into the binary, which is brittle outside the repo and can break releases/containers. Consider gating this behind a build tag or dev-only build flags, or move the path wiring to the build pipeline (e.g., env/config) so production artifacts don’t depend on.cache.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wasmvm.go` at line 3, The cgo LDFLAGS line currently embeds a repo-local rpath (-Wl,-rpath,${SRCDIR}/.cache/lib) into the binary; remove or conditionalize this hardcoded rpath in the wasmvm.go cgo directive and instead supply the runtime library path via a dev-only build tag or external build flag/environment variable. Specifically, replace the direct -Wl,-rpath usage with one of: (a) move the cgo directive into a separate file with a build tag like // +build dev so production builds omit it, or (b) read an environment/config variable at build time and inject the rpath via the build pipeline (go build -ldflags or CI wrapper) so the main wasmvm.go (the cgo LDFLAGS directive) no longer bakes in ${SRCDIR}/.cache/lib. Ensure the symbol to edit is the cgo LDFLAGS comment at the top of the wasmvm.go file.x/market/client/rest/rest.go (1)
9-9: Remove the commented import.The commented import serves no purpose since all code that would use it is also commented out. Remove it entirely rather than leaving it as a comment.
🧹 Proposed cleanup
- // "pkg.akt.dev/node/v2/x/market/query"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/market/client/rest/rest.go` at line 9, Remove the unused commented import line "// \"pkg.akt.dev/node/v2/x/market/query\"" from rest.go; locate the import block in the file (the commented line appears near the top of x/market/client/rest/rest.go) and delete that commented import so the import section contains only active imports and no leftover commented code.x/escrow/query/querier.go (1)
3-12: Consider removing commented-out dead code.This entire file consists of commented-out code that appears to be a placeholder (
NewQuerierreturnsnil). If the querier is no longer needed in v2, consider removing this file entirely rather than maintaining dead code.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/escrow/query/querier.go` around lines 3 - 12, This file contains only commented-out imports and a stub NewQuerier function; remove the dead code by deleting the file (querier.go) entirely if the querier is not required in v2, or alternatively restore a real implementation by uncommenting and implementing NewQuerier and required imports (codec.LegacyAmino, sdk types, keeper.Keeper) if the querier should exist; prefer deletion to avoid keeping commented placeholders.x/wasm/simulation/decoder.go (1)
1-17: Remove misleading commented-out placeholder code.This file contains only a package declaration and commented-out code that references
types.Minterandtypes.MinterKey— these are mint module types, not wasm types. This appears to be copy-pasted boilerplate that doesn't apply to the wasm module.Consider either:
- Implementing the actual wasm decoder with appropriate wasm types, or
- Removing this file entirely until the decoder is needed
Leaving misleading commented code adds confusion for future maintainers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/wasm/simulation/decoder.go` around lines 1 - 17, Remove the misleading commented-out boilerplate in NewDecodeStore that references mint module symbols (types.Minter, types.MinterKey); either delete this decoder.go file entirely if no wasm-specific simulation decoder is needed, or replace the commented block with a real wasm decoder implementation using actual wasm module types and keys (update the NewDecodeStore function and any referenced symbols accordingly) so the file no longer contains cross-module placeholder code.x/oracle/simulation/genesis.go (1)
9-15: Clarify “randomized” behavior (or actually randomize).The function currently uses
DefaultParams()only, so the “randomized” wording is misleading. Either randomize params or adjust the comment to reflect deterministic behavior.💡 Possible clarification
-// RandomizedGenState generates a random GenesisState for supply +// RandomizedGenState generates a default GenesisState for simulation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/oracle/simulation/genesis.go` around lines 9 - 15, The function RandomizedGenState is misleading because it always uses DefaultParams(); either make it actually random or change the comment to reflect deterministic behavior. Fix option A: implement randomized params by creating a new helper (e.g., RandomParams or RandomizedParams) that uses simState.Rand to produce varied types.Params values and assign them to takeGenesis.Params before marshalling. Fix option B: if you want deterministic behavior, change the function name/comment from "RandomizedGenState" to indicate default/deterministic genesis and keep using types.DefaultParams(); update the docstring accordingly. Ensure you update references to RandomizedGenState or the params helper (types.DefaultParams, takeGenesis, simState.GenState) consistently.x/oracle/simulation/decoder.go (1)
3-17: Remove or implement the commented-out decoder.Leaving a fully commented implementation makes maintenance harder and can mislead readers. If this is intended, wire it in; otherwise delete the file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/oracle/simulation/decoder.go` around lines 3 - 17, The file contains a fully commented decoder; either restore and export it or delete the file. To restore: re-enable the NewDecodeStore(codec.Codec) func so it returns func(kvA, kvB kv.Pair) string, import bytes, fmt and use the passed-in cdc to MustUnmarshal kvA.Value/kvB.Value into types.Minter variables and return fmt.Sprintf("%v\n%v", minterA, minterB) when bytes.Equal(kvA.Key, types.MinterKey) (panic for default), ensuring NewDecodeStore, types.Minter, and types.MinterKey are referenced and imports cleaned. Alternatively, remove the commented block/file entirely if the decoder is not needed.make/cosmwasm.mk (1)
1-7: Fail fast when the contract directory is missing.A quick preflight check gives a clearer error than a Docker failure on an empty mount.
🧩 Suggested guard
build-contract-%: + `@test` -d "$(ROOT_DIR)/contracts/$*" || (echo "contract not found: $(ROOT_DIR)/contracts/$*" >&2; exit 1) mkdir -p $(AKASH_DEVCACHE)/cosmwasm/$* docker run --rm -v "$(ROOT_DIR)/contracts/$*":/code \ -v "$(AKASH_DEVCACHE)/cosmwasm/$*":/target \ --mount type=volume,source=registry_cache,target=/usr/local/cargo/registry \ $(COSMWASM_OPTIMIZER_IMAGE)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@make/cosmwasm.mk` around lines 1 - 7, The build-contract-% target should fail fast if the contract source directory is missing; add a preflight check in the build-contract-% recipe that verifies the directory "$(ROOT_DIR)/contracts/$*" exists and is non-empty and, if not, prints a clear error and exits non-zero. Update the build-contract-% target (in cosmwasm.mk) to perform this test before mkdir and docker run so you get an informative message rather than a Docker empty-mount failure.make/init.mk (1)
70-74: Add guard to prevent emptyWASMVM_MODfrom silently breaking version resolution.The
WASMVM_MODvalue is used directly to extractWASMVM_VERSION, which is later used in download URLs inmake/setup-cache.mk. If the grep returns empty, the resulting version variable would be malformed, causing silent failures at build time. Addinggrep -m1and an early error check improves robustness:Suggested guard
-WASMVM_MOD := $(shell $(GO) list -m -f '{{ .Path }}' all | grep github.com/CosmWasm/wasmvm) -WASMVM_VERSION := $(shell $(GO) list -mod=readonly -m -f '{{ .Version }}' $(WASMVM_MOD)) +WASMVM_MOD := $(shell $(GO) list -m -f '{{ .Path }}' all | grep -m1 'github.com/CosmWasm/wasmvm') +ifeq ($(strip $(WASMVM_MOD)),) +$(error "wasmvm module not found in go.mod/go.work") +endif +WASMVM_VERSION := $(shell $(GO) list -mod=readonly -m -f '{{ .Version }}' $(WASMVM_MOD))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@make/init.mk` around lines 70 - 74, The Makefile currently assigns WASMVM_MOD and then computes WASMVM_VERSION directly, which can silently break if WASMVM_MOD is empty; update the WASMVM_MOD assignment to use grep -m1 to pick the first match and add an explicit guard right after (check if WASMVM_MOD is empty) that prints a clear error and exits non‑zero; then only compute WASMVM_VERSION when WASMVM_MOD is nonempty so downstream targets (e.g., make/setup-cache.mk) won’t get a malformed version.tests/upgrade/workers_test.go (1)
123-134: Drop the second UnpackAny to avoid redundant decoding.
You already unpack intosdk.AccountIand type-assert tosdk.ModuleAccountI; the second UnpackAny intomaccis unnecessary and can be removed.♻️ Suggested cleanup
macc, ok := acc.(sdk.ModuleAccountI) require.True(t, ok) - - err = encCfg.InterfaceRegistry.UnpackAny(qResp.Account, &macc) - require.NoError(t, err)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/upgrade/workers_test.go` around lines 123 - 134, The second call to encCfg.InterfaceRegistry.UnpackAny(qResp.Account, &macc) is redundant because you already UnpackAny into var acc (sdk.AccountI) and then type-asserted it to macc (sdk.ModuleAccountI); remove the redundant UnpackAny line and keep the initial UnpackAny into acc, the type assertion macc,ok and the require checks so the test still validates decoding and the module account type.make/setup-cache.mk (1)
62-80: Prevent stale wasmvm libs whenWASMVM_VERSIONchanges.
The verification marker doesn’t encode the version, so a version bump can keep old libs “verified.” Consider versioning the marker (or storing a version file as a prerequisite).♻️ One way to version the marker
-.PHONY: wasmvm-libs-verify -wasmvm-libs-verify: - @$(MAKE) -s $(AKASH_DEVCACHE_LIB)/.wasmvm_verified +WASMVM_VERIFIED := $(AKASH_DEVCACHE_LIB)/.wasmvm_verified_$(WASMVM_VERSION) + +.PHONY: wasmvm-libs-verify +wasmvm-libs-verify: + @$(MAKE) -s $(WASMVM_VERIFIED)-$(AKASH_DEVCACHE_LIB)/.wasmvm_verified: $(patsubst %, $(AKASH_DEVCACHE_LIB)/%,$(WASMVM_LIBS)) $(AKASH_DEVCACHE_LIB)/wasmvm_checksums.txt +$(WASMVM_VERIFIED): $(patsubst %, $(AKASH_DEVCACHE_LIB)/%,$(WASMVM_LIBS)) $(AKASH_DEVCACHE_LIB)/wasmvm_checksums.txt cd $(AKASH_DEVCACHE_LIB) && sha256sum -c --ignore-missing wasmvm_checksums.txt `@touch` $@🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@make/setup-cache.mk` around lines 62 - 80, The verification marker .wasmvm_verified should be tied to WASMVM_VERSION so old artifacts aren't considered verified after a version bump; change the marker to include the version (e.g. use .wasmvm_verified-$(WASMVM_VERSION) or create a wasmvm_version file and make .wasmvm_verified depend on it) and update the targets that reference $(AKASH_DEVCACHE_LIB)/.wasmvm_verified (the rule for $(AKASH_DEVCACHE_LIB)/.wasmvm_verified, wasmvm-libs-verify, and wasmvm-libs) so the checksum verification and the touch use the versioned marker/filename and the new prerequisite (wasmvm_checksums.txt plus version) to force re-verification when WASMVM_VERSION changes; ensure the patsubst and touch calls reference the new marker name and that any rm -f lines clean the old versioned marker if necessary.Makefile (1)
24-26: Duplicate variable definitions.Several variables are defined twice:
GOMODis defined on line 24 and again on line 35 with the same default valueBUILD_TAGSis defined on line 25 but unconditionally overwritten on line 41GORELEASER_STRIP_FLAGSis defined on line 26 and again on line 45The second definitions on lines 35, 41, and 45 will override the first ones. If this is intentional (e.g., line 25 is legacy), consider removing the unused definitions to avoid confusion.
♻️ Suggested cleanup
-BUILD_TAGS ?= osusergo,netgo,hidraw,ledger -GORELEASER_STRIP_FLAGS ?= - - ifeq ($(IS_MAINNET), true) ifeq ($(IS_PREREL), false) IS_STABLE := true endif endif -GOMOD ?= readonly - ifneq ($(UNAME_OS),Darwin)Also applies to: 35-45
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 24 - 26, Remove the duplicate Makefile variable definitions to avoid accidental overrides: keep a single authoritative definition for GOMOD, BUILD_TAGS, and GORELEASER_STRIP_FLAGS and delete the redundant declarations (the later re-definitions that override the earlier ones); if the later values are the intended defaults, delete the earlier lines, otherwise move the intended values into the single kept declaration for each variable (GOMOD, BUILD_TAGS, GORELEASER_STRIP_FLAGS) and ensure BUILD_TAGS is not unconditionally overwritten by removing or conditionalizing the second assignment._run/node/Makefile (1)
1-5: Consider adding stub targets or excluding this file from minphony lint.
If checkmake runs on_run/node/Makefile, the missingall/clean/testphony targets may keep warning. Adding stubs or excluding this snippet from that rule would silence lint.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@_run/node/Makefile` around lines 1 - 5, This Makefile triggers checkmake minphony warnings because common phony targets like all, clean, and test are missing; either add stub phony targets (e.g., .PHONY: all clean test and empty rule bodies for all, clean, test) alongside the existing clean-node, or mark this file to be skipped by the linter (e.g., update the linter exclude list) so checkmake/minphony ignores it; update the file containing the phony list or the lint config accordingly and ensure you reference the existing clean-node target when adding the stubs to avoid duplicate definitions.x/market/simulation/operations.go (1)
229-293: Consider removing or restoring the commented-out simulation code.
SimulateMsgCloseLeasecontains ~60 lines of commented-out implementation. If this code is no longer needed, consider removing it to improve readability. If it's intended to be restored, consider tracking it with a TODO or issue reference.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/market/simulation/operations.go` around lines 229 - 293, The SimulateMsgCloseLease function contains a large commented-out simulation implementation (around the MsgCloseLease flow); either remove the dead-comment block to clean up the file or restore it properly and mark it with a TODO/issue reference. If removing: delete the commented lines inside SimulateMsgCloseLease so the function only returns the NoOpMsg using v1.ModuleName and types.MsgCloseLease. If restoring: uncomment and ensure symbols used (SimulateMsgCloseLease, types.NewMsgCloseLease/MsgCloseLease, moduletestutil.MakeTestEncodingConfig, simtestutil.GenSignedMockTx, app.SimDeliver) compile and add a TODO comment linking an issue that explains why the simulation is active. Ensure no remaining large commented blocks remain in the function.app/app.go (2)
137-143: Silent failure on type assertion for wasm options.If
appOpts.Get("wasm")returns a non-nil value that is not[]wasmkeeper.Option, the type assertion will silently fail andwasmOptswill remain empty. Consider logging a warning when the type doesn't match.♻️ Proposed improvement
var wasmOpts []wasmkeeper.Option if val := appOpts.Get("wasm"); val != nil { - if vl, valid := val.([]wasmkeeper.Option); valid { + if vl, valid := val.([]wasmkeeper.Option); valid { wasmOpts = append(wasmOpts, vl...) + } else { + logger.Warn("wasm option provided but type assertion failed", "type", fmt.Sprintf("%T", val)) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/app.go` around lines 137 - 143, The type assertion for wasm options can silently fail leaving wasmOpts empty; update the block that calls appOpts.Get("wasm") to check the assertion boolean and when val != nil and the assertion to []wasmkeeper.Option is false, emit a warning (e.g., app.Logger().Warn or fmt.Printf) indicating the unexpected type and include the actual type via fmt.Sprintf("%T", val) and the key "wasm", otherwise append the asserted slice to wasmOpts as before; ensure you reference wasmOpts, appOpts.Get("wasm"), and wasmkeeper.Option so reviewers can locate the change.
158-170: Config overrides afterReadNodeConfigmay surprise operators.The hardcoded values for
MemoryCacheSize,SmartQueryGasLimit, andContractDebugModeare applied after reading the node config, effectively overriding any custom settings from the config file. This is likely intentional for security, but consider documenting this behavior or adding a log message.The values themselves are reasonable:
- 100 MB memory cache is conservative
- 3M gas limit for queries is appropriate
- Debug mode disabled is correct for production
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/app.go` around lines 158 - 170, The code currently unconditionally overrides values from wasm.ReadNodeConfig by setting wasmConfig.MemoryCacheSize, wasmConfig.SmartQueryGasLimit, and wasmConfig.ContractDebugMode after loading the config; change this to only apply these hardcoded defaults when the loaded config has zero/unspecified values (e.g., if wasmConfig.MemoryCacheSize == 0 then set to 100, etc.), and add a concise log message noting when you override each field (use the same variables and functions: wasm.ReadNodeConfig, wasmConfig.MemoryCacheSize, wasmConfig.SmartQueryGasLimit, wasmConfig.ContractDebugMode) so operators are not surprised by silent overrides.
Comment on lines
+471
to
+505
| wOpts := make([]wasmkeeper.Option, 0, len(wasmOpts)+1) | ||
|
|
||
| wOpts = append(wOpts, wasmkeeper.WithMessageHandlerDecorator( | ||
| app.Keepers.Akash.Wasm.NewMsgFilterDecorator(), | ||
| )) | ||
|
|
||
| wOpts = append(wOpts, wasmOpts...) | ||
|
|
||
| // The last arguments can contain custom message handlers and custom query handlers | ||
| // if we want to allow any custom callbacks | ||
| wasmCapabilities := wasmkeeper.BuiltInCapabilities() | ||
| wasmCapabilities = append(wasmCapabilities, "akash") | ||
|
|
||
| wasmKeeper := wasmkeeper.NewKeeper( | ||
| cdc, | ||
| runtime.NewKVStoreService(app.keys[wasmtypes.StoreKey]), | ||
| app.Keepers.Cosmos.Acct, | ||
| app.Keepers.Cosmos.Bank, | ||
| *app.Keepers.Cosmos.Staking, | ||
| distrkeeper.NewQuerier(app.Keepers.Cosmos.Distr), | ||
| app.Keepers.Cosmos.IBC.ChannelKeeper, | ||
| app.Keepers.Cosmos.IBC.ChannelKeeper, | ||
| app.Keepers.Cosmos.IBC.ChannelKeeperV2, | ||
| app.Keepers.Cosmos.Transfer, | ||
| bApp.MsgServiceRouter(), | ||
| bApp.GRPCQueryRouter(), | ||
| wasmDir, | ||
| wasmConfig, | ||
| wasmtypes.VMConfig{}, | ||
| wasmCapabilities, | ||
| authtypes.NewModuleAddress(govtypes.ModuleName).String(), | ||
| wOpts..., | ||
| ) | ||
| app.Keepers.Cosmos.Wasm = &wasmKeeper | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the go.mod file and check wasmd version
fd go.mod -type fRepository: akash-network/node
Length of output: 232
🏁 Script executed:
# Once we find go.mod, let's check the wasmd dependency version
head -100 go.mod | grep -i wasmdRepository: akash-network/node
Length of output: 96
🌐 Web query:
wasmd v0.61.6 wasmkeeper NewKeeper function signature
💡 Result:
In wasmd v0.61.6 (x/wasm/keeper), the NewKeeper function signature is: [1]
func NewKeeper(
cdc codec.Codec,
storeService corestoretypes.KVStoreService,
accountKeeper types.AccountKeeper,
bankKeeper types.BankKeeper,
stakingKeeper types.StakingKeeper,
distrKeeper types.DistributionKeeper,
ics4Wrapper types.ICS4Wrapper,
channelKeeper types.ChannelKeeper,
channelKeeperV2 types.ChannelKeeperV2,
portSource types.ICS20TransferPortSource,
router MessageRouter,
_ GRPCQueryRouter,
homeDir string,
nodeConfig types.NodeConfig,
vmConfig types.VMConfig,
availableCapabilities []string,
authority string,
opts ...Option,
) KeeperSources: [1] (pkg.go.dev)
Citations:
Fix parameter 7 in wasmkeeper.NewKeeper call: pass ics4Wrapper instead of ChannelKeeper.
The wasmd v0.61.6 signature expects parameter 7 to be types.ICS4Wrapper, but the code passes app.Keepers.Cosmos.IBC.ChannelKeeper (which is types.ChannelKeeper). Parameter 8 is correctly set to ChannelKeeper. Passing the wrong interface type at position 7 will cause a compilation error or type mismatch.
Additionally, verify that parameter 14 (wasmConfig) matches the expected types.NodeConfig type and that parameter 6's use of distrkeeper.NewQuerier() is intentional rather than passing the keeper directly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/types/app.go` around lines 471 - 505, The call to wasmkeeper.NewKeeper is
passing app.Keepers.Cosmos.IBC.ChannelKeeper as the 7th argument but the wasmd
v0.61.6 signature requires an ICS4Wrapper there; replace the 7th parameter with
your IBC ICS4Wrapper instance (e.g., the ICS4Wrapper provided by your IBC
keeper, not ChannelKeeper) when constructing wasmKeeper, ensure the 8th argument
remains ChannelKeeper, and confirm wasmConfig matches the expected NodeConfig
type and that using distrkeeper.NewQuerier(...) for parameter 6 was intentional
versus passing the Distr keeper directly.
Comment on lines
+1
to
+3
| use cosmwasm_schema::write_api; | ||
| use akt_pyth_oracle::msg::{ExecuteMsg, InstantiateMsg, QueryMsg}; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the Cargo.toml for the crate name and lib configuration
echo "=== Cargo.toml in contracts/price-oracle ==="
cat contracts/price-oracle/Cargo.toml
echo ""
echo "=== Checking for lib name override ==="
grep -A 5 "^\[lib\]" contracts/price-oracle/Cargo.toml || echo "No [lib] section found"
echo ""
echo "=== Searching for akt_pyth_oracle references ==="
rg "akt_pyth_oracle|akt-pyth-oracle" --max-count 20Repository: akash-network/node
Length of output: 1163
Fix crate path for message imports—this will cause a compile error.
Line 2 imports akt_pyth_oracle::msg, but the crate is named price-oracle (Rust crate name price_oracle). No lib name override or dependency provides akt_pyth_oracle, so this import will fail at compile time.
🔧 Proposed fix
-use akt_pyth_oracle::msg::{ExecuteMsg, InstantiateMsg, QueryMsg};
+use price_oracle::msg::{ExecuteMsg, InstantiateMsg, QueryMsg};📝 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.
Suggested change
| use cosmwasm_schema::write_api; | |
| use akt_pyth_oracle::msg::{ExecuteMsg, InstantiateMsg, QueryMsg}; | |
| use cosmwasm_schema::write_api; | |
| use price_oracle::msg::{ExecuteMsg, InstantiateMsg, QueryMsg}; | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contracts/price-oracle/src/bin/schema.rs` around lines 1 - 3, The import path
uses the wrong crate name: replace the use statement that references
akt_pyth_oracle (use akt_pyth_oracle::msg::{ExecuteMsg, InstantiateMsg,
QueryMsg};) with the correct crate name for this project (the Rust crate name
price_oracle) so the line imports from price_oracle::msg::{ExecuteMsg,
InstantiateMsg, QueryMsg}; ensuring the module symbols ExecuteMsg,
InstantiateMsg, and QueryMsg resolve and the schema generator builds
successfully.
Comment on lines
+106
to
+122
| let config = CONFIG.load(deps.storage)?; | ||
|
|
||
| // Check if sufficient fee was paid (CosmWasm 3.x uses Uint256 for coin amounts) | ||
| let sent_amount = info | ||
| .funds | ||
| .iter() | ||
| .find(|coin| coin.denom == "uakt") | ||
| .map(|coin| coin.amount) | ||
| .unwrap_or_else(Uint256::zero); | ||
|
|
||
| if sent_amount < config.update_fee { | ||
| return Err(ContractError::InsufficientFunds { | ||
| required: config.update_fee.to_string(), | ||
| sent: sent_amount.to_string(), | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
Price updates are unauthenticated. Any sender paying the fee can set the price, which is risky for an oracle. If updates must be trusted, restrict to admin/allowlist or verify signed oracle data.
🔒 Suggested guard (if updates are permissioned)
let config = CONFIG.load(deps.storage)?;
+ // Only admin (or allowlisted feeders) may publish prices
+ if info.sender != config.admin {
+ return Err(ContractError::Unauthorized {});
+ }
+
// Check if sufficient fee was paid (CosmWasm 3.x uses Uint256 for coin amounts)
let sent_amount = info🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contracts/price-oracle/src/contract.rs` around lines 106 - 122, The update
handler currently only checks payment (using CONFIG, config.update_fee,
info.funds and sent_amount) and allows any sender to set the price; restrict
updates by adding an authorization check: after loading CONFIG, verify
info.sender is allowed (compare against an admin/owner field in CONFIG or
consult an ALLOWED_UPDATERS storage item) or validate a signed price payload
(accept a signature and signer pubkey and verify the signature before applying
the update); if unauthorized return a new ContractError variant (e.g.,
UnauthorizedUpdater) instead of proceeding. Ensure you update CONFIG/storage
types and error enum accordingly and perform the check before accepting
funds/setting the price.
Comment on lines
+133
to
+140
| // Check staleness | ||
| let current_time = env.block.time.seconds() as i64; | ||
| if current_time - publish_time > MAX_STALENESS { | ||
| return Err(ContractError::StalePriceData { | ||
| current_time, | ||
| publish_time, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Reject future publish_time to avoid freezing updates. A future timestamp passes the staleness check and then blocks subsequent updates until chain time catches up.
🕒 Suggested validation
let current_time = env.block.time.seconds() as i64;
+ if publish_time > current_time {
+ return Err(ContractError::InvalidPriceData {
+ reason: format!(
+ "Publish time {} is in the future (current {})",
+ publish_time, current_time
+ ),
+ });
+ }
if current_time - publish_time > MAX_STALENESS {📝 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.
Suggested change
| // Check staleness | |
| let current_time = env.block.time.seconds() as i64; | |
| if current_time - publish_time > MAX_STALENESS { | |
| return Err(ContractError::StalePriceData { | |
| current_time, | |
| publish_time, | |
| }); | |
| } | |
| // Check staleness | |
| let current_time = env.block.time.seconds() as i64; | |
| if publish_time > current_time { | |
| return Err(ContractError::InvalidPriceData { | |
| reason: format!( | |
| "Publish time {} is in the future (current {})", | |
| publish_time, current_time | |
| ), | |
| }); | |
| } | |
| if current_time - publish_time > MAX_STALENESS { | |
| return Err(ContractError::StalePriceData { | |
| current_time, | |
| publish_time, | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contracts/price-oracle/src/contract.rs` around lines 133 - 140, Add an
explicit check that rejects a future publish_time before the staleness
calculation: in the same validation block where current_time, publish_time and
MAX_STALENESS are used, if publish_time > current_time return an error (e.g. a
new ContractError variant like FuturePublishTime or reuse an existing error
type) instead of letting a future timestamp pass the staleness check; update any
callers/tests to expect this rejection.
Comment on lines
+284
to
+381
| #[test] | ||
| fn test_instantiate_with_provided_id() { | ||
| let mut deps = mock_dependencies(); | ||
| let msg = InstantiateMsg { | ||
| admin: "admin".to_string(), | ||
| update_fee: Uint256::from(1000u128), | ||
| price_feed_id: "0xabc123def456".to_string(), | ||
| }; | ||
| let info = message_info(&deps.api.addr_make("creator"), &[]); | ||
| let env = mock_env(); | ||
|
|
||
| let res = instantiate(deps.as_mut(), env.clone(), info, msg).unwrap(); | ||
| assert_eq!(4, res.attributes.len()); | ||
|
|
||
| let config: ConfigResponse = | ||
| from_json(&query(deps.as_ref(), env, QueryMsg::GetConfig {}).unwrap()).unwrap(); | ||
| assert_eq!("admin", config.admin); | ||
| assert_eq!("0xabc123def456", config.price_feed_id); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_update_price_feed() { | ||
| let mut deps = mock_dependencies(); | ||
|
|
||
| let config = Config { | ||
| admin: deps.api.addr_make("admin"), | ||
| update_fee: Uint256::from(1000u128), | ||
| price_feed_id: "0xtest123".to_string(), | ||
| }; | ||
| CONFIG.save(&mut deps.storage, &config).unwrap(); | ||
|
|
||
| let price_feed = PriceFeed::new(); | ||
| PRICE_FEED.save(&mut deps.storage, &price_feed).unwrap(); | ||
|
|
||
| let env = mock_env(); | ||
|
|
||
| let update_msg = ExecuteMsg::UpdatePriceFeed { | ||
| price: Uint128::new(123000000), | ||
| conf: Uint128::new(1000000), | ||
| expo: -8, | ||
| publish_time: env.block.time.seconds() as i64, | ||
| }; | ||
| let info = message_info(&deps.api.addr_make("updater"), &[coin(1000, "uakt")]); | ||
| let res = execute(deps.as_mut(), env.clone(), info, update_msg).unwrap(); | ||
| assert_eq!(5, res.attributes.len()); | ||
|
|
||
| let price: PriceResponse = | ||
| from_json(&query(deps.as_ref(), env, QueryMsg::GetPrice {}).unwrap()).unwrap(); | ||
| assert_eq!(Uint128::new(123000000), price.price); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_update_fee() { | ||
| let mut deps = mock_dependencies(); | ||
|
|
||
| let config = Config { | ||
| admin: deps.api.addr_make("admin"), | ||
| update_fee: Uint256::from(1000u128), | ||
| price_feed_id: "0xtest123".to_string(), | ||
| }; | ||
| CONFIG.save(&mut deps.storage, &config).unwrap(); | ||
|
|
||
| let msg = ExecuteMsg::UpdateFee { | ||
| new_fee: Uint256::from(2000u128), | ||
| }; | ||
| let info = message_info(&deps.api.addr_make("admin"), &[]); | ||
| let res = execute(deps.as_mut(), mock_env(), info, msg).unwrap(); | ||
| assert_eq!(2, res.attributes.len()); | ||
|
|
||
| let config: ConfigResponse = | ||
| from_json(&query(deps.as_ref(), mock_env(), QueryMsg::GetConfig {}).unwrap()) | ||
| .unwrap(); | ||
| assert_eq!(Uint256::from(2000u128), config.update_fee); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_query_price_feed_id() { | ||
| let mut deps = mock_dependencies(); | ||
|
|
||
| let config = Config { | ||
| admin: deps.api.addr_make("admin"), | ||
| update_fee: Uint256::from(1000u128), | ||
| price_feed_id: "0xabc123def456".to_string(), | ||
| }; | ||
| CONFIG.save(&mut deps.storage, &config).unwrap(); | ||
|
|
||
| let response: PriceFeedIdResponse = from_json( | ||
| &query( | ||
| deps.as_ref(), | ||
| mock_env(), | ||
| QueryMsg::GetPriceFeedId {}, | ||
| ) | ||
| .unwrap(), | ||
| ) | ||
| .unwrap(); | ||
|
|
||
| assert_eq!("0xabc123def456", response.price_feed_id); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check cosmwasm_std version and mock dependency helpers
echo "=== cosmwasm_std version in Cargo.toml ==="
rg -n "cosmwasm-std|cosmwasm_std" -g 'Cargo.toml'
echo -e "\n=== Instances of mock_dependencies usage ==="
rg -n "mock_dependencies" --type=rust -C 2
echo -e "\n=== Check for AkashQuery definition/usage ==="
rg -n "AkashQuery" --type=rust
echo -e "\n=== Check for custom mock helpers ==="
rg -n "mock_dependencies_with_custom|create_mock_environment" --type=rustRepository: akash-network/node
Length of output: 3781
🏁 Script executed:
#!/bin/bash
# Look for custom mock_dependencies implementation or generic helpers
echo "=== Search for mock_dependencies implementation in querier.rs or tests ==="
rg -A 10 "fn mock_dependencies" --type=rust
echo -e "\n=== Check querier.rs for custom mock helpers ==="
head -50 contracts/price-oracle/src/querier.rs
echo -e "\n=== Check if there's a test utilities module ==="
fd "test|mock" --type=f contracts/price-oracle/src/
echo -e "\n=== Full test function signatures ==="
rg -B 2 "fn test_instantiate_with_provided_id" -A 15 --type=rustRepository: akash-network/node
Length of output: 2546
🏁 Script executed:
#!/bin/bash
# Check for Cargo.lock to see actual resolved versions
echo "=== Check Cargo.lock for cosmwasm_std ==="
rg "cosmwasm-std" Cargo.lock | head -20
echo -e "\n=== Check if tests are configured in Cargo.toml ==="
cat contracts/price-oracle/Cargo.toml | grep -A 10 "\[\[test"
echo -e "\n=== Check for any CI/build scripts ==="
find . -maxdepth 2 -type f \( -name "*.yml" -o -name "*.yaml" -o -name "Makefile" \) | head -10
echo -e "\n=== Look for build output or test configuration ==="
ls -la contracts/price-oracle/ | grep -E "target|\.cargo|build"Repository: akash-network/node
Length of output: 513
🏁 Script executed:
#!/bin/bash
# Check CI workflows for test execution
echo "=== GitHub workflows for Rust testing ==="
find .github/workflows -type f \( -name "*.yml" -o -name "*.yaml" \) | head -5
echo -e "\n=== Check if any workflow runs contract tests ==="
rg "price-oracle|contract.*test|cargo test" .github/workflows/
echo -e "\n=== Check Makefile for test targets ==="
grep -n "test\|contract" Makefile | head -20
echo -e "\n=== Check mock_dependencies generic signature in cosmwasm_std 3.0.2 docs ==="
# Look at the actual function call to understand type inference
rg -B 5 -A 5 "let mut deps = mock_dependencies" contracts/price-oracle/src/contract.rsRepository: akash-network/node
Length of output: 1936
🏁 Script executed:
#!/bin/bash
# Check if tests are marked as ignored or disabled
echo "=== Check test attributes ==="
rg "#\[.*test.*\]|#\[ignore\]" -B 1 contracts/price-oracle/src/contract.rs | grep -E "test|ignore"
echo -e "\n=== Check if price-oracle is built/tested at all ==="
grep -r "price-oracle" Makefile .github/workflows/ 2>/dev/null || echo "No matches found"
echo -e "\n=== Try to understand mock_dependencies type signature ==="
# Check imports to see full context
head -30 contracts/price-oracle/src/contract.rsRepository: akash-network/node
Length of output: 1353
🏁 Script executed:
#!/bin/bash
# Check the actual function signatures to see if they're generic
echo "=== Check instantiate function signature ==="
rg -A 5 "pub fn instantiate" contracts/price-oracle/src/contract.rs
echo -e "\n=== Check execute function signature ==="
rg -A 5 "pub fn execute" contracts/price-oracle/src/contract.rs
echo -e "\n=== Check if there's any generic parameter handling ==="
rg "impl.*<.*>.*\|where " contracts/price-oracle/src/contract.rs | head -20
echo -e "\n=== Verify no type parameter on mock_dependencies call ==="
rg "mock_dependencies\(" contracts/price-oracle/src/contract.rsRepository: akash-network/node
Length of output: 1276
🏁 Script executed:
#!/bin/bash
# Check cosmwasm_std 3.0.2 documentation for mock_dependencies signature
# and verify if there's a way the type inference could work
echo "=== Check the complete test module structure ==="
sed -n '279,290p' contracts/price-oracle/src/contract.rs
echo -e "\n=== Verify no hidden trait bounds or type specifications ==="
rg -B 20 "fn test_instantiate_with_provided_id" contracts/price-oracle/src/contract.rs | grep -E "impl|type|where|generic|CustomQuery"
echo -e "\n=== Check if cosmwasm_std re-exports or wraps mock_dependencies ==="
rg "mock_dependencies|MockQuerier" contracts/price-oracle/src/ -n
echo -e "\n=== Most importantly: do these tests actually compile? Check for build config ==="
cat contracts/price-oracle/Cargo.tomlRepository: akash-network/node
Length of output: 1996
🏁 Script executed:
#!/bin/bash
# Final verification: try to understand if these tests are meant to work
# Check if price-oracle is even part of the workspace
echo "=== Check workspace configuration ==="
cat Cargo.toml | grep -A 20 "\[workspace\]"
echo -e "\n=== Check if price-oracle has its own lib/test setup ==="
ls -la contracts/price-oracle/src/
echo -e "\n=== Search for any build scripts or test configuration specific to price-oracle ==="
find contracts/price-oracle -name "build.rs" -o -name "lib.rs" -o -name "tests"Repository: akash-network/node
Length of output: 859
🏁 Script executed:
#!/bin/bash
# Find the root Cargo.toml
echo "=== Find Cargo.toml files ==="
find . -maxdepth 3 -name "Cargo.toml" -type f
echo -e "\n=== Check the actual root Cargo.toml ==="
cat ./Cargo.toml 2>/dev/null || echo "Not at root"
echo -e "\n=== Check if price-oracle is in a subdir Cargo.toml ==="
cat contracts/Cargo.toml 2>/dev/null | head -30 || echo "No contracts/Cargo.toml"
echo -e "\n=== Check lib.rs to see what's exported ==="
cat contracts/price-oracle/src/lib.rsRepository: akash-network/node
Length of output: 399
🏁 Script executed:
#!/bin/bash
# Final confirmation: Check cosmwasm_std 3.0.2 documentation behavior
# by looking at how mock_dependencies would be inferred
echo "=== Verify the type mismatch is real by checking function parameter types ==="
rg "instantiate\(deps" contracts/price-oracle/src/contract.rs -A 1
echo -e "\n=== Check if tests even have a chance of running ==="
rg "#\[cfg\(test\)\]" contracts/price-oracle/src/contract.rs
echo -e "\n=== Final check: cosmwasm_std 3.0.2 mock_dependencies returns what? ==="
# Search for any conditional compilation or feature flags that might enable AkashQuery support
rg "cfg\(|feature" contracts/price-oracle/src/contract.rsRepository: akash-network/node
Length of output: 620
🏁 Script executed:
#!/bin/bash
# Final verification: Check if mock_dependencies can be used with AkashQuery
# by examining if there's a generic version available in cosmwasm_std 3.0.2
echo "=== Check cosmwasm_std documentation for generics ==="
rg "mock_dependencies" --type=rust -B 2 -A 8 | head -50
echo -e "\n=== Check if cw-multi-test provides the proper mocking for custom queries ==="
rg "cw.multi.test|cw_multi_test" contracts/price-oracle/Cargo.toml
echo -e "\n=== Verify that deps.as_mut() would have the wrong type ==="
# This confirms the type mismatch is real by checking the actual call
sed -n '286,295p' contracts/price-oracle/src/contract.rsRepository: akash-network/node
Length of output: 4040
🏁 Script executed:
#!/bin/bash
# Check if there's a generic version of mock_dependencies available in 3.0.2
echo "=== Check if mock_dependencies has a generic turbofish form ==="
rg "mock_dependencies::" --type=rust
echo -e "\n=== Verify no type annotation exists anywhere ==="
rg "deps.*:.*OwnedDeps|DepsMut.*<" contracts/price-oracle/src/contract.rs | grep test
echo -e "\n=== Check what cw-multi-test provides instead ==="
grep -A 5 "cw-multi-test" contracts/price-oracle/Cargo.tomlRepository: akash-network/node
Length of output: 299
These tests will not compile. In cosmwasm_std 3.0.2, mock_dependencies() without type parameters defaults to OwnedDeps<_, _, MockQuerier<Empty>>, but all contract functions require DepsMut<AkashQuery>. To fix, use mock_dependencies::<AkashQuery>() with turbofish syntax, or refactor contract functions to accept generic DepsMut<C: CustomQuery> for test compatibility.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contracts/price-oracle/src/contract.rs` around lines 284 - 381, Tests fail to
compile because mock_dependencies() is producing OwnedDeps with the default
MockQuerier<Empty> while the contract functions expect DepsMut<AkashQuery>;
update the tests to call mock_dependencies::<AkashQuery>() (turbofish) wherever
mock_dependencies() is used (e.g., in test_instantiate_with_provided_id,
test_update_price_feed, test_update_fee, test_query_price_feed_id) so the
returned deps match the contract's DepsMut<AkashQuery>; alternatively, if you
prefer a library change, make instantiate, execute, and query accept generic
DepsMut<C: CustomQuery> so tests can use the default mock_dependencies, but the
minimal fix is to replace mock_dependencies() with
mock_dependencies::<AkashQuery>() in these tests.
Comment on lines
21
to
24
| .PHONY: test-integration | ||
| test-integration: | ||
| $(GO_TEST) -v -tags="e2e.integration" $(TEST_MODULES) | ||
| $(GO_TEST) -v -tags="e2e.integration" -ldflags '$(ldflags)' $(TEST_MODULES) | ||
|
|
There was a problem hiding this comment.
test-integration should depend on wasmvm-libs like other test targets.
Without this, integration tests can run before wasmvm libs are available.
🛠️ Suggested fix
-.PHONY: test-integration
-test-integration:
+.PHONY: test-integration
+test-integration: wasmvm-libs
$(GO_TEST) -v -tags="e2e.integration" -ldflags '$(ldflags)' $(TEST_MODULES)📝 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.
Suggested change
| .PHONY: test-integration | |
| test-integration: | |
| $(GO_TEST) -v -tags="e2e.integration" $(TEST_MODULES) | |
| $(GO_TEST) -v -tags="e2e.integration" -ldflags '$(ldflags)' $(TEST_MODULES) | |
| .PHONY: test-integration | |
| test-integration: wasmvm-libs | |
| $(GO_TEST) -v -tags="e2e.integration" -ldflags '$(ldflags)' $(TEST_MODULES) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@make/test-integration.mk` around lines 21 - 24, The test-integration Make
target lacks the dependency on wasmvm-libs so integration tests may run before
the wasmvm artifacts are built; update the Makefile by adding wasmvm-libs as a
prerequisite for the test-integration target (i.e., change the target header for
test-integration to list wasmvm-libs before the recipe) so that the wasmvm-libs
target is executed/built prior to running the $(GO_TEST) command.
Comment on lines
+1
to
+12
| package wasm | ||
|
|
||
| import ( | ||
| types "pkg.akt.dev/go/node/oracle/v1" | ||
| ) | ||
|
|
||
| const ( | ||
| // StoreKey represents storekey of wasm module | ||
| StoreKey = types.StoreKey | ||
| // ModuleName represents current module name | ||
| ModuleName = types.ModuleName | ||
| ) |
There was a problem hiding this comment.
Package name mismatch: declared as wasm but file is in x/oracle directory.
The file is located at x/oracle/alias.go but declares package wasm instead of package oracle. This will cause a compilation error since Go requires the package name to match the directory name (with some exceptions).
Additionally, the comments on lines 8 and 10 reference "wasm module" but the types are imported from pkg.akt.dev/go/node/oracle/v1.
Proposed fix
-package wasm
+package oracle
import (
types "pkg.akt.dev/go/node/oracle/v1"
)
const (
- // StoreKey represents storekey of wasm module
+ // StoreKey represents storekey of oracle module
StoreKey = types.StoreKey
- // ModuleName represents current module name
+ // ModuleName represents oracle module name
ModuleName = types.ModuleName
)📝 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.
Suggested change
| package wasm | |
| import ( | |
| types "pkg.akt.dev/go/node/oracle/v1" | |
| ) | |
| const ( | |
| // StoreKey represents storekey of wasm module | |
| StoreKey = types.StoreKey | |
| // ModuleName represents current module name | |
| ModuleName = types.ModuleName | |
| ) | |
| package oracle | |
| import ( | |
| types "pkg.akt.dev/go/node/oracle/v1" | |
| ) | |
| const ( | |
| // StoreKey represents storekey of oracle module | |
| StoreKey = types.StoreKey | |
| // ModuleName represents oracle module name | |
| ModuleName = types.ModuleName | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@x/oracle/alias.go` around lines 1 - 12, The file declares the wrong package
name and misleading comments: change the package declaration from "wasm" to
"oracle" and update the two comments referencing "wasm module" to refer to the
oracle module; leave the constants StoreKey and ModuleName (and the import types
from pkg.akt.dev/go/node/oracle/v1) intact so StoreKey and ModuleName continue
to alias types.StoreKey and types.ModuleName.
| @@ -0,0 +1,39 @@ | |||
| package wasm | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "^package " x/oracle -g '*.go'Repository: akash-network/node
Length of output: 456
Fix package name mismatch (compile-time blocker).
x/oracle/genesis.go declares package wasm, which won't compile under the x/oracle directory. This should be package oracle. This issue also affects x/oracle/module.go and x/oracle/alias.go.
✅ Suggested fix
-package wasm
+package oracle📝 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.
Suggested change
| package wasm | |
| package oracle |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@x/oracle/genesis.go` at line 1, The package declaration in x/oracle Go files
is incorrect: change the top-line package from "wasm" to "oracle" in
x/oracle/genesis.go, x/oracle/module.go, and x/oracle/alias.go so the package
name matches the directory (replace any "package wasm" with "package oracle");
after updating, ensure imports and any package-qualified references still
compile (run go build/go vet) and adjust any internal references to the package
name if present.
Comment on lines
+19
to
+22
| func (k Querier) PriceFeedConfig(ctx context.Context, request *types.QueryPriceFeedConfigRequest) (*types.QueryPriceFeedConfigResponse, error) { | ||
| //TODO implement me | ||
| panic("implement me") | ||
| } |
There was a problem hiding this comment.
Replace panic with a proper gRPC “unimplemented” error.
A panic in a query handler can crash the gRPC server. Return codes.Unimplemented instead.
🛠️ Suggested fix
func (k Querier) PriceFeedConfig(ctx context.Context, request *types.QueryPriceFeedConfigRequest) (*types.QueryPriceFeedConfigResponse, error) {
- //TODO implement me
- panic("implement me")
+ return nil, status.Errorf(codes.Unimplemented, "PriceFeedConfig is not implemented")
}📝 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.
Suggested change
| func (k Querier) PriceFeedConfig(ctx context.Context, request *types.QueryPriceFeedConfigRequest) (*types.QueryPriceFeedConfigResponse, error) { | |
| //TODO implement me | |
| panic("implement me") | |
| } | |
| func (k Querier) PriceFeedConfig(ctx context.Context, request *types.QueryPriceFeedConfigRequest) (*types.QueryPriceFeedConfigResponse, error) { | |
| return nil, status.Errorf(codes.Unimplemented, "PriceFeedConfig is not implemented") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@x/oracle/keeper/grpc_query.go` around lines 19 - 22, The PriceFeedConfig
query handler currently panics; replace the panic in Querier.PriceFeedConfig
with returning a gRPC unimplemented error (use status.Errorf with
codes.Unimplemented and a concise message like "PriceFeedConfig not
implemented") so the server doesn't crash; also add the necessary imports for
"google.golang.org/grpc/codes" and "google.golang.org/grpc/status" (or use
status.Error) and ensure the function returns
(*types.QueryPriceFeedConfigResponse)(nil) along with the error.
Comment on lines
+63
to
+169
| // FilterMessage applies Phase 1 filtering rules | ||
| func (k *keeper) FilterMessage(sctx sdk.Context, contractAddr sdk.AccAddress, msg wasmvmtypes.CosmosMsg, | ||
| ) error { | ||
| // ALLOW Bank messages (with restrictions) | ||
| if msg.Bank != nil { | ||
| return k.filterBankMessage(sctx, msg.Bank) | ||
| } | ||
|
|
||
| // BLOCK Staking messages | ||
| if msg.Staking != nil { | ||
| return errorsmod.Wrap( | ||
| sdkerrors.ErrUnauthorized, | ||
| "Staking operations not allowed", | ||
| ) | ||
| } | ||
|
|
||
| // BLOCK Distribution messages | ||
| if msg.Distribution != nil { | ||
| return errorsmod.Wrap( | ||
| sdkerrors.ErrUnauthorized, | ||
| "Distribution operations not allowed", | ||
| ) | ||
| } | ||
|
|
||
| // BLOCK Governance messages | ||
| if msg.Gov != nil { | ||
| return errorsmod.Wrap( | ||
| sdkerrors.ErrUnauthorized, | ||
| "Governance operations not allowed", | ||
| ) | ||
| } | ||
|
|
||
| // BLOCK IBC messages | ||
| if msg.IBC != nil { | ||
| return errorsmod.Wrap( | ||
| sdkerrors.ErrUnauthorized, | ||
| "IBC messages not allowed", | ||
| ) | ||
| } | ||
|
|
||
| if msg.IBC2 != nil { | ||
| return errorsmod.Wrap( | ||
| sdkerrors.ErrUnauthorized, | ||
| "IBC2 messages not allowed", | ||
| ) | ||
| } | ||
|
|
||
| // BLOCK Custom messages (no Akash bindings) | ||
| if msg.Custom != nil { | ||
| return errorsmod.Wrap( | ||
| sdkerrors.ErrUnauthorized, | ||
| "Custom messages not allowed", | ||
| ) | ||
| } | ||
|
|
||
| // BLOCK Any messages (no Akash bindings) | ||
| if msg.Any != nil { | ||
| return errorsmod.Wrap( | ||
| sdkerrors.ErrUnauthorized, | ||
| "Any messages not allowed", | ||
| ) | ||
| } | ||
|
|
||
| // ALLOW Wasm messages (contract-to-contract calls) | ||
| if msg.Wasm != nil { | ||
| // Wasm execute/instantiate allowed | ||
| return nil | ||
| } | ||
|
|
||
| // BLOCK unknown/unhandled message types | ||
| return errorsmod.Wrap( | ||
| sdkerrors.ErrUnauthorized, | ||
| "Unknown message type not allowed", | ||
| ) | ||
| } | ||
|
|
||
| // filterBankMessage applies restrictions to bank operations | ||
| func (k *keeper) filterBankMessage(sctx sdk.Context, msg *wasmvmtypes.BankMsg) error { | ||
| // Allow send with restrictions | ||
| if msg.Send != nil { | ||
| params := k.GetParams(sctx) | ||
|
|
||
| // Block transfers to critical addresses | ||
| for _, addr := range params.BlockedAddresses { | ||
| if addr == msg.Send.ToAddress { | ||
| return errorsmod.Wrapf( | ||
| sdkerrors.ErrUnauthorized, | ||
| "Transfers to %s blocked (critical address)", | ||
| msg.Send.ToAddress, | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| // Transfers to regular addresses allowed | ||
| return nil | ||
| } | ||
|
|
||
| // Deny burns | ||
| if msg.Burn != nil { | ||
| return errorsmod.Wrapf( | ||
| sdkerrors.ErrUnauthorized, | ||
| "Burn is not allowed", | ||
| ) | ||
| } | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
Block unknown BankMsg variants instead of silently allowing them.
If a BankMsg has no Send/Burn (or a new variant is introduced), this currently returns nil, which effectively bypasses Phase‑1 restrictions for that variant. Safer default is to reject unknown bank operations.
🛡️ Suggested fix
// Deny burns
if msg.Burn != nil {
return errorsmod.Wrapf(
sdkerrors.ErrUnauthorized,
"Burn is not allowed",
)
}
- return nil
+ return errorsmod.Wrap(
+ sdkerrors.ErrUnauthorized,
+ "Unsupported bank message type",
+ )
}📝 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.
Suggested change
| // FilterMessage applies Phase 1 filtering rules | |
| func (k *keeper) FilterMessage(sctx sdk.Context, contractAddr sdk.AccAddress, msg wasmvmtypes.CosmosMsg, | |
| ) error { | |
| // ALLOW Bank messages (with restrictions) | |
| if msg.Bank != nil { | |
| return k.filterBankMessage(sctx, msg.Bank) | |
| } | |
| // BLOCK Staking messages | |
| if msg.Staking != nil { | |
| return errorsmod.Wrap( | |
| sdkerrors.ErrUnauthorized, | |
| "Staking operations not allowed", | |
| ) | |
| } | |
| // BLOCK Distribution messages | |
| if msg.Distribution != nil { | |
| return errorsmod.Wrap( | |
| sdkerrors.ErrUnauthorized, | |
| "Distribution operations not allowed", | |
| ) | |
| } | |
| // BLOCK Governance messages | |
| if msg.Gov != nil { | |
| return errorsmod.Wrap( | |
| sdkerrors.ErrUnauthorized, | |
| "Governance operations not allowed", | |
| ) | |
| } | |
| // BLOCK IBC messages | |
| if msg.IBC != nil { | |
| return errorsmod.Wrap( | |
| sdkerrors.ErrUnauthorized, | |
| "IBC messages not allowed", | |
| ) | |
| } | |
| if msg.IBC2 != nil { | |
| return errorsmod.Wrap( | |
| sdkerrors.ErrUnauthorized, | |
| "IBC2 messages not allowed", | |
| ) | |
| } | |
| // BLOCK Custom messages (no Akash bindings) | |
| if msg.Custom != nil { | |
| return errorsmod.Wrap( | |
| sdkerrors.ErrUnauthorized, | |
| "Custom messages not allowed", | |
| ) | |
| } | |
| // BLOCK Any messages (no Akash bindings) | |
| if msg.Any != nil { | |
| return errorsmod.Wrap( | |
| sdkerrors.ErrUnauthorized, | |
| "Any messages not allowed", | |
| ) | |
| } | |
| // ALLOW Wasm messages (contract-to-contract calls) | |
| if msg.Wasm != nil { | |
| // Wasm execute/instantiate allowed | |
| return nil | |
| } | |
| // BLOCK unknown/unhandled message types | |
| return errorsmod.Wrap( | |
| sdkerrors.ErrUnauthorized, | |
| "Unknown message type not allowed", | |
| ) | |
| } | |
| // filterBankMessage applies restrictions to bank operations | |
| func (k *keeper) filterBankMessage(sctx sdk.Context, msg *wasmvmtypes.BankMsg) error { | |
| // Allow send with restrictions | |
| if msg.Send != nil { | |
| params := k.GetParams(sctx) | |
| // Block transfers to critical addresses | |
| for _, addr := range params.BlockedAddresses { | |
| if addr == msg.Send.ToAddress { | |
| return errorsmod.Wrapf( | |
| sdkerrors.ErrUnauthorized, | |
| "Transfers to %s blocked (critical address)", | |
| msg.Send.ToAddress, | |
| ) | |
| } | |
| } | |
| // Transfers to regular addresses allowed | |
| return nil | |
| } | |
| // Deny burns | |
| if msg.Burn != nil { | |
| return errorsmod.Wrapf( | |
| sdkerrors.ErrUnauthorized, | |
| "Burn is not allowed", | |
| ) | |
| } | |
| return nil | |
| } | |
| // filterBankMessage applies restrictions to bank operations | |
| func (k *keeper) filterBankMessage(sctx sdk.Context, msg *wasmvmtypes.BankMsg) error { | |
| // Allow send with restrictions | |
| if msg.Send != nil { | |
| params := k.GetParams(sctx) | |
| // Block transfers to critical addresses | |
| for _, addr := range params.BlockedAddresses { | |
| if addr == msg.Send.ToAddress { | |
| return errorsmod.Wrapf( | |
| sdkerrors.ErrUnauthorized, | |
| "Transfers to %s blocked (critical address)", | |
| msg.Send.ToAddress, | |
| ) | |
| } | |
| } | |
| // Transfers to regular addresses allowed | |
| return nil | |
| } | |
| // Deny burns | |
| if msg.Burn != nil { | |
| return errorsmod.Wrapf( | |
| sdkerrors.ErrUnauthorized, | |
| "Burn is not allowed", | |
| ) | |
| } | |
| return errorsmod.Wrap( | |
| sdkerrors.ErrUnauthorized, | |
| "Unsupported bank message type", | |
| ) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@x/wasm/keeper/msg_filter.go` around lines 63 - 169, The filterBankMessage
function currently returns nil for unknown BankMsg variants (when msg.Send and
msg.Burn are both nil), which silently allows future or unexpected bank
operations; update filterBankMessage to reject any BankMsg that is not
explicitly handled by returning an unauthorized error (use errorsmod.Wrapf with
sdkerrors.ErrUnauthorized and a clear message like "Unknown bank message variant
not allowed"), keeping the existing handling for Send (including using
k.GetParams and params.BlockedAddresses and the blocked-address check) and Burn
intact, so only Send/Burn are permitted and all other variants cause an error.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
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.md