WIP feat(go/wasm): implement wasm params #158
Conversation
WalkthroughAdds v1 wasm and oracle proto schemas and Go wiring (codec, keys, params, msgs), extensive wasm CLI commands, parsing helpers, tests and testdata, and updates multiple Go module dependencies and replace directives. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ 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: 7
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
go/node/wasm/v1/event.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/genesis.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/params.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/paramsmsg.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/query.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/query.pb.gw.gois excluded by!**/*.pb.gw.gogo/node/wasm/v1/service.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (11)
buf.yaml(1 hunks)go/node/wasm/v1/codec.go(1 hunks)go/node/wasm/v1/key.go(1 hunks)go/node/wasm/v1/msgs.go(1 hunks)go/node/wasm/v1/params.go(1 hunks)proto/node/akash/wasm/v1/event.proto(1 hunks)proto/node/akash/wasm/v1/genesis.proto(1 hunks)proto/node/akash/wasm/v1/params.proto(1 hunks)proto/node/akash/wasm/v1/paramsmsg.proto(1 hunks)proto/node/akash/wasm/v1/query.proto(1 hunks)proto/node/akash/wasm/v1/service.proto(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
go/node/wasm/v1/params.go (1)
go/node/wasm/v1/params.pb.go (3)
Params(28-30)Params(34-34)Params(35-37)
go/node/wasm/v1/msgs.go (4)
go/node/wasm/v1/paramsmsg.pb.go (3)
MsgUpdateParams(31-38)MsgUpdateParams(42-42)MsgUpdateParams(43-45)go/node/wasm/v1/params.pb.go (3)
Params(28-30)Params(34-34)Params(35-37)go/node/wasm/v1/codec.go (1)
ModuleCdc(19-19)go/node/wasm/v1/key.go (1)
RouterKey(11-11)
go/node/wasm/v1/codec.go (2)
go/node/wasm/v1/paramsmsg.pb.go (3)
MsgUpdateParams(31-38)MsgUpdateParams(42-42)MsgUpdateParams(43-45)go/node/wasm/v1/key.go (1)
ModuleName(5-5)
🪛 Buf (1.59.0)
proto/node/akash/wasm/v1/genesis.proto
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/wasm/v1/params.proto
5-5: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/wasm/v1/service.proto
4-4: import "cosmos/msg/v1/msg.proto": file does not exist
(COMPILE)
proto/node/akash/wasm/v1/query.proto
5-5: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/wasm/v1/paramsmsg.proto
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/wasm/v1/event.proto
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: coverage
- GitHub Check: test
- GitHub Check: breakage
- GitHub Check: go
- GitHub Check: proto
🔇 Additional comments (6)
buf.yaml (1)
59-59: LGTM! Dependency addition is appropriate.The wasmd dependency is correctly added to enable protobuf imports for the wasm module implementation.
go/node/wasm/v1/key.go (1)
1-16: LGTM! Standard module key definitions.The constants and prefix function follow Cosmos SDK conventions correctly.
proto/node/akash/wasm/v1/genesis.proto (1)
9-17: LGTM! Standard genesis state definition.The GenesisState message follows Cosmos SDK conventions with proper field annotations.
proto/node/akash/wasm/v1/params.proto (1)
10-17: LGTM! Well-structured parameter definition.The Params message is properly defined with appropriate address annotations and serialization tags.
go/node/wasm/v1/codec.go (1)
22-36: Codec wiring looks solid.Helper registrations line up with Cosmos SDK expectations and should unblock Msg service wiring.
proto/node/akash/wasm/v1/event.proto (1)
4-5: Fix missing gogoproto import dependency.The gogoproto dependency is missing from the module's dependency graph. While
buf.yamlreferencesgo/vendor/github.com/cosmos/gogoprotoas a local vendor path, this directory does not exist on the filesystem, and gogoproto is not declared as an external dependency inbuf.lock. This will cause buf compilation to fail for the 123+ proto files (includingevent.proto) that import from gogoproto.Add gogoproto as an external dependency in
buf.yaml'sdepssection (e.g.,- buf.build/cosmos/gogoproto), then runbuf mod updateto resolve and lock the dependency.
| func (m *MsgUpdateParams) GetSigners() []sdk.AccAddress { | ||
| addr, _ := sdk.AccAddressFromBech32(m.Authority) | ||
| return []sdk.AccAddress{addr} | ||
| } |
There was a problem hiding this comment.
Handle Bech32 decode failure in GetSigners.
GetSigners drops the error from sdk.AccAddressFromBech32, returning a nil signer when Authority is malformed. BaseApp collects signers before ValidateBasic, so an invalid address will slip through until signature verification, producing non-deterministic panics/nil checks. Decode must be strict.
Apply this diff to make the failure explicit:
func (m *MsgUpdateParams) GetSigners() []sdk.AccAddress {
- addr, _ := sdk.AccAddressFromBech32(m.Authority)
- return []sdk.AccAddress{addr}
+ addr, err := sdk.AccAddressFromBech32(m.Authority)
+ if err != nil {
+ panic(err)
+ }
+ return []sdk.AccAddress{addr}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (m *MsgUpdateParams) GetSigners() []sdk.AccAddress { | |
| addr, _ := sdk.AccAddressFromBech32(m.Authority) | |
| return []sdk.AccAddress{addr} | |
| } | |
| func (m *MsgUpdateParams) GetSigners() []sdk.AccAddress { | |
| addr, err := sdk.AccAddressFromBech32(m.Authority) | |
| if err != nil { | |
| panic(err) | |
| } | |
| return []sdk.AccAddress{addr} | |
| } |
🤖 Prompt for AI Agents
In go/node/wasm/v1/msgs.go around lines 28 to 31, GetSigners currently ignores
the error from sdk.AccAddressFromBech32 which can return a nil address; change
it to strictly handle the decode error by checking the error and panicking (with
a clear message including the invalid authority and the decode error) when
decoding fails, otherwise return the parsed address in the slice.
| // ParamKeyTable for take module | ||
| // Deprecated: now params can be accessed on key `0x01` on the take store. |
There was a problem hiding this comment.
Fix copy-paste errors in comments.
The comments reference "take module" and "take store" but this is the wasm module.
Apply this diff:
-// ParamKeyTable for take module
-// Deprecated: now params can be accessed on key `0x01` on the take store.
+// ParamKeyTable for wasm module
+// Deprecated: now params can be accessed on key `0x01` on the wasm store.📝 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.
| // ParamKeyTable for take module | |
| // Deprecated: now params can be accessed on key `0x01` on the take store. | |
| // ParamKeyTable for wasm module | |
| // Deprecated: now params can be accessed on key `0x01` on the wasm store. |
🤖 Prompt for AI Agents
In go/node/wasm/v1/params.go around lines 9 to 10, the comment incorrectly
references the "take module" and "take store"; update the comment to reference
the wasm module and wasm store instead. Replace "take module" with "wasm module"
and "take store" with "wasm store" so the deprecated notice reads that params
can now be accessed on key `0x01` on the wasm store.
| func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs { | ||
| return paramtypes.ParamSetPairs{} | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify empty ParamSetPairs implementation.
The ParamSetPairs method returns an empty set. Ensure this is intentional for the newer SDK parameter mechanism, as the deprecated comment on ParamKeyTable suggests migration to a new approach.
Run the following script to check how other modules in this codebase implement ParamSetPairs:
🏁 Script executed:
#!/bin/bash
# Search for ParamSetPairs implementations in the codebase
rg -n -A5 'func.*ParamSetPairs.*paramtypes.ParamSetPairs' --type goLength of output: 3570
I need to gather more information about the wasm module to determine if the empty ParamSetPairs is intentional.
🏁 Script executed:
#!/bin/bash
# Check the Params struct definition in wasm module
echo "=== Wasm Params struct definition ==="
rg -n -B5 -A20 'type Params struct' go/node/wasm/v1/params.go
echo -e "\n=== Check for parameter key definitions in wasm ==="
rg -n -i 'key.*=' go/node/wasm/v1/params.go | head -20
echo -e "\n=== Check for ParamKeyTable method ==="
rg -n 'ParamKeyTable' go/node/wasm/v1/params.goLength of output: 328
🏁 Script executed:
#!/bin/bash
# Check the Params struct definition in wasm module
echo "=== Wasm Params struct definition ==="
rg -n -B5 -A20 'type Params struct' go/node/wasm/v1/params.go
echo -e "\n=== Check for parameter key definitions in wasm ==="
rg -n -i 'const.*[kK]ey' go/node/wasm/v1/params.go
echo -e "\n=== Check for ParamKeyTable method ==="
rg -n 'ParamKeyTable' go/node/wasm/v1/params.go
echo -e "\n=== Full params.go file (first 50 lines) ==="
head -50 go/node/wasm/v1/params.goLength of output: 795
🏁 Script executed:
#!/bin/bash
# Find Params struct definition - may be in proto file
echo "=== Search for Params struct or message in wasm module ==="
fd -e go -e proto . go/node/wasm | xargs rg -l 'message Params|type Params'
echo -e "\n=== Check proto files in wasm/v1 ==="
find go/node/wasm/v1 -name '*.proto' -exec echo "File: {}" \; -exec grep -A10 'message Params' {} \;
echo -e "\n=== Full params.go file ==="
cat go/node/wasm/v1/params.goLength of output: 692
🏁 Script executed:
#!/bin/bash
# Extract Params struct from generated proto file
echo "=== Params struct from params.pb.go ==="
rg -n -A15 'type Params struct' go/node/wasm/v1/params.pb.go
echo -e "\n=== Check if there are any proto source files ==="
find go/node/wasm -name '*.proto'
echo -e "\n=== Look for params definition in any proto ==="
find go/node/wasm -name '*.proto' -exec cat {} \; | grep -A20 'message Params'Length of output: 853
Add BlockedAddresses to ParamSetPairs and fix module name in comment.
The Params struct contains a BlockedAddresses field, but ParamSetPairs() returns an empty set. This is inconsistent with other modules in the codebase (take, staking, deployment, market), which all register their fields as parameters. The BlockedAddresses parameter should either be registered in ParamSetPairs() with validation, or removed from the struct if not intended as a module parameter.
Additionally, the comment on line 9 incorrectly references "take module" instead of "wasm module".
| func (p Params) Validate() error { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add validation for blocked_addresses parameter.
The Validate method performs no checks on the blocked_addresses field. Consider validating that each address in the list is a valid Cosmos address format to prevent invalid configuration.
Apply this diff to add validation:
+import (
+ sdk "github.com/cosmos/cosmos-sdk/types"
+ sdkerrors "cosmossdk.io/errors"
+)
+
func (p Params) Validate() error {
+ for _, addr := range p.BlockedAddresses {
+ if _, err := sdk.AccAddressFromBech32(addr); err != nil {
+ return sdkerrors.Wrapf(err, "invalid blocked address: %s", addr)
+ }
+ }
return nil
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In go/node/wasm/v1/params.go around lines 23-25, the Validate method currently
returns nil and does not validate the blocked_addresses parameter; update
Validate to iterate over the blocked addresses and for each call
sdk.AccAddressFromBech32 (from github.com/cosmos/cosmos-sdk/types) to verify
Bech32 address format, returning a descriptive error (e.g. fmt.Errorf("invalid
blocked address %q: %w", addr, err)) on the first failure; ensure you add
necessary imports (sdk and fmt) and keep behavior of allowing an empty list
intact.
| // Query defines the gRPC querier service of the take package. | ||
| service Query { | ||
| // Params returns the total set of minting parameters. |
There was a problem hiding this comment.
Fix copy-paste errors in comments.
The comments reference "take package" and "minting parameters" but this is the wasm module.
Apply this diff:
-// Query defines the gRPC querier service of the take package.
+// Query defines the gRPC querier service of the wasm package.
service Query {
- // Params returns the total set of minting parameters.
+ // Params returns the total set of wasm parameters.
rpc Params(QueryParamsRequest) returns (QueryParamsResponse) {🤖 Prompt for AI Agents
In proto/node/akash/wasm/v1/query.proto around lines 24 to 26, the service and
RPC comments were copy-pasted incorrectly referencing the "take package" and
"minting parameters"; update the comments to correctly describe the wasm module
and related queries (e.g., "Query defines the gRPC querier service for the wasm
module." and change "Params returns the total set of minting parameters." to a
description appropriate for the Params RPC in the wasm module, such as "Params
returns the wasm module parameters."). Ensure wording is concise and matches
module semantics.
|
|
||
| option go_package = "pkg.akt.dev/go/node/wasm/v1"; | ||
|
|
||
| // Msg defines the market Msg service |
There was a problem hiding this comment.
Fix copy-paste error in comment.
The comment references "market Msg service" but this is the wasm module service.
Apply this diff:
-// Msg defines the market Msg service
+// Msg defines the wasm Msg service📝 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.
| // Msg defines the market Msg service | |
| // Msg defines the wasm Msg service |
🤖 Prompt for AI Agents
In proto/node/akash/wasm/v1/service.proto around line 10, the top-of-file
comment incorrectly reads "Msg defines the market Msg service" due to a
copy-paste error; update the comment to reference the wasm module instead (e.g.,
"Msg defines the wasm Msg service" or "Msg defines the WASM Msg service") so it
correctly describes this file's service.
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
go/cli/go.sumis excluded by!**/*.sumgo/cli/testdata/wasm/burner.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/cyberpunk.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/hackatom.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/hackatom_42.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/hackatom_420.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/ibc_reflect.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/queue.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/reflect_1_1.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/reflect_1_5.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/reflect_2_0.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/replier.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/staking.wasmis excluded by!**/*.wasm
📒 Files selected for processing (14)
go/cli/authz_tx.go(2 hunks)go/cli/flags/flags.go(1 hunks)go/cli/go.mod(13 hunks)go/cli/gov_tx.go(5 hunks)go/cli/query.go(1 hunks)go/cli/testdata/wasm/contracts.go(1 hunks)go/cli/testdata/wasm/download_releases.sh(1 hunks)go/cli/tx.go(1 hunks)go/cli/wasm_parse.go(1 hunks)go/cli/wasm_query.go(1 hunks)go/cli/wasm_tx.go(1 hunks)go/cli/wasm_tx_test.go(1 hunks)go/node/client/v1beta3/client.go(2 hunks)go/node/client/v1beta3/query.go(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-16T10:32:01.602Z
Learnt from: cloud-j-luna
Repo: akash-network/chain-sdk PR: 118
File: go/cli/tx.go:48-63
Timestamp: 2025-10-16T10:32:01.602Z
Learning: In the Akash chain-sdk CLI (Go), when `cctx.Offline` is true, nil clients are intentionally stored in the context using `ContextTypeClient` and `ContextTypeQueryClient` keys. Operations that support offline mode should check for `ErrContextValueNotSet` when retrieving clients from context using `ClientFromContext()` or `QueryClientFromContext()`, rather than using the `Must*` variants which panic on nil values.
Applied to files:
go/node/client/v1beta3/query.go
🧬 Code graph analysis (9)
go/cli/tx.go (1)
go/cli/wasm_tx.go (1)
GetTxWasmCmd(20-41)
go/cli/wasm_tx_test.go (4)
go/sdkutil/address.go (1)
MustAccAddressFromBech32(7-13)go/cli/testdata/wasm/contracts.go (1)
ChecksumHackatom(12-12)go/cli/gov_tx.go (1)
GetTxGovWasmProposalStoreAndInstantiateContractCmd(781-906)go/cli/wasm_tx.go (1)
GetTxWasmStoreCodeCmd(44-75)
go/cli/authz_tx.go (2)
go/cli/context.go (1)
MustClientFromContext(47-54)go/cli/flags/flags.go (8)
FlagAllowedMsgKeys(275-275)FlagAllowedRawMsgs(276-276)FlagMaxFunds(278-278)FlagMaxCalls(277-277)FlagExpiration(94-94)FlagAllowAllMsgs(279-279)FlagNoTokenTransfer(280-280)AddTxFlagsToCmd(336-373)
go/cli/wasm_parse.go (1)
go/cli/flags/flags.go (11)
FlagSource(263-263)FlagBuilder(264-264)FlagCodeHash(265-265)FlagInstantiateByAnyOfAddress(273-273)FlagInstantiateByAddress(272-272)FlagInstantiateByEverybody(270-270)FlagInstantiateNobody(271-271)FlagAmount(163-163)FlagLabel(262-262)FlagAdmin(266-266)FlagNoAdmin(267-267)
go/cli/wasm_tx.go (3)
go/cli/tx.go (1)
TxPersistentPreRunE(13-52)go/cli/context.go (1)
MustClientFromContext(47-54)go/cli/flags/flags.go (10)
AddTxFlagsToCmd(336-373)FlagInstantiateByEverybody(270-270)FlagInstantiateNobody(271-271)FlagInstantiateByAddress(272-272)FlagInstantiateByAnyOfAddress(273-273)FlagAmount(163-163)FlagLabel(262-262)FlagAdmin(266-266)FlagNoAdmin(267-267)FlagFixMsg(268-268)
go/cli/wasm_query.go (4)
go/cli/query.go (1)
QueryPersistentPreRunE(12-38)go/cli/context.go (1)
MustLightClientFromContext(75-82)go/cli/utils.go (1)
ReadPageRequest(193-225)go/cli/flags/flags.go (2)
AddQueryFlagsToCmd(323-333)AddPaginationFlagsToCmd(382-389)
go/cli/query.go (1)
go/cli/wasm_query.go (1)
GetQueryWasmCmd(25-49)
go/node/client/v1beta3/query.go (1)
go/node/client/v1beta3/client.go (1)
QueryClient(40-63)
go/cli/gov_tx.go (2)
go/cli/flags/flags.go (15)
FlagAuthority(186-186)FlagAmount(163-163)FlagLabel(262-262)FlagAdmin(266-266)FlagNoAdmin(267-267)FlagFixMsg(268-268)FlagUnpinCode(274-274)FlagSource(263-263)FlagBuilder(264-264)FlagCodeHash(265-265)AddTxFlagsToCmd(336-373)FlagTitle(69-69)FlagSummary(71-71)FlagDeposit(92-92)FlagExpedite(281-281)go/cli/cctx.go (1)
GetClientTxContext(81-84)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: coverage
- GitHub Check: test
| amountStr, err := cmd.Flags().GetString(cflags.FlagAmount) | ||
| if err != nil { | ||
| return fmt.Errorf("amount: %s", err) | ||
| } | ||
| amount, err := sdk.ParseCoinsNormalized(amountStr) | ||
| if err != nil { | ||
| return fmt.Errorf("amount: %s", err) | ||
| } | ||
| label, err := cmd.Flags().GetString(cflags.FlagLabel) | ||
| if err != nil { | ||
| return fmt.Errorf("label: %s", err) | ||
| } | ||
| if label == "" { | ||
| return errors.New("label is required on all contracts") | ||
| } | ||
| adminStr, err := cmd.Flags().GetString(cflags.FlagAdmin) | ||
| if err != nil { | ||
| return fmt.Errorf("admin: %s", err) | ||
| } | ||
| noAdmin, err := cmd.Flags().GetBool(cflags.FlagNoAdmin) | ||
| if err != nil { | ||
| return fmt.Errorf("no-admin: %s", err) | ||
| } | ||
|
|
||
| // ensure sensible admin is set (or explicitly immutable) | ||
| if adminStr == "" && !noAdmin { | ||
| return errors.New("you must set an admin or explicitly pass --no-admin to make it immutable (wasmd issue #719)") | ||
| } | ||
| if adminStr != "" && noAdmin { | ||
| return errors.New("you set an admin and passed --no-admin, those cannot both be true") | ||
| } | ||
|
|
||
| if adminStr != "" { | ||
| addr, err := sdk.AccAddressFromBech32(adminStr) | ||
| if err != nil { | ||
| info, err := clientCtx.Keyring.Key(adminStr) | ||
| if err != nil { | ||
| return fmt.Errorf("admin %s", err) | ||
| } | ||
| admin, err := info.GetAddress() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| adminStr = admin.String() | ||
| } else { | ||
| adminStr = addr.String() | ||
| } | ||
| } | ||
|
|
||
| storeAndInstantiateMsg := wtypes.MsgStoreAndInstantiateContract{ | ||
| Authority: authority, | ||
| WASMByteCode: storeCodeMsg.WASMByteCode, | ||
| InstantiatePermission: storeCodeMsg.InstantiatePermission, | ||
| UnpinCode: unpinCode, | ||
| Source: source, | ||
| Builder: builder, | ||
| CodeHash: codeHash, | ||
| Admin: adminStr, | ||
| Label: label, | ||
| Msg: []byte(args[1]), | ||
| Funds: amount, | ||
| } |
There was a problem hiding this comment.
Honor optional --amount in store-instantiate proposal
sdk.ParseCoinsNormalized rejects the default empty string, so this path fails with invalid decimal coin expression: "" unless callers always pass --amount, even though the help text marks it optional. Gate the parsing on the flag being provided and leave funds nil when it isn’t set.
Apply this diff:
amountStr, err := cmd.Flags().GetString(cflags.FlagAmount)
if err != nil {
return fmt.Errorf("amount: %s", err)
}
- amount, err := sdk.ParseCoinsNormalized(amountStr)
- if err != nil {
- return fmt.Errorf("amount: %s", err)
- }
+ var amount sdk.Coins
+ if strings.TrimSpace(amountStr) != "" {
+ amount, err = sdk.ParseCoinsNormalized(amountStr)
+ if err != nil {
+ return fmt.Errorf("amount: %s", err)
+ }
+ }📝 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.
| amountStr, err := cmd.Flags().GetString(cflags.FlagAmount) | |
| if err != nil { | |
| return fmt.Errorf("amount: %s", err) | |
| } | |
| amount, err := sdk.ParseCoinsNormalized(amountStr) | |
| if err != nil { | |
| return fmt.Errorf("amount: %s", err) | |
| } | |
| label, err := cmd.Flags().GetString(cflags.FlagLabel) | |
| if err != nil { | |
| return fmt.Errorf("label: %s", err) | |
| } | |
| if label == "" { | |
| return errors.New("label is required on all contracts") | |
| } | |
| adminStr, err := cmd.Flags().GetString(cflags.FlagAdmin) | |
| if err != nil { | |
| return fmt.Errorf("admin: %s", err) | |
| } | |
| noAdmin, err := cmd.Flags().GetBool(cflags.FlagNoAdmin) | |
| if err != nil { | |
| return fmt.Errorf("no-admin: %s", err) | |
| } | |
| // ensure sensible admin is set (or explicitly immutable) | |
| if adminStr == "" && !noAdmin { | |
| return errors.New("you must set an admin or explicitly pass --no-admin to make it immutable (wasmd issue #719)") | |
| } | |
| if adminStr != "" && noAdmin { | |
| return errors.New("you set an admin and passed --no-admin, those cannot both be true") | |
| } | |
| if adminStr != "" { | |
| addr, err := sdk.AccAddressFromBech32(adminStr) | |
| if err != nil { | |
| info, err := clientCtx.Keyring.Key(adminStr) | |
| if err != nil { | |
| return fmt.Errorf("admin %s", err) | |
| } | |
| admin, err := info.GetAddress() | |
| if err != nil { | |
| return err | |
| } | |
| adminStr = admin.String() | |
| } else { | |
| adminStr = addr.String() | |
| } | |
| } | |
| storeAndInstantiateMsg := wtypes.MsgStoreAndInstantiateContract{ | |
| Authority: authority, | |
| WASMByteCode: storeCodeMsg.WASMByteCode, | |
| InstantiatePermission: storeCodeMsg.InstantiatePermission, | |
| UnpinCode: unpinCode, | |
| Source: source, | |
| Builder: builder, | |
| CodeHash: codeHash, | |
| Admin: adminStr, | |
| Label: label, | |
| Msg: []byte(args[1]), | |
| Funds: amount, | |
| } | |
| amountStr, err := cmd.Flags().GetString(cflags.FlagAmount) | |
| if err != nil { | |
| return fmt.Errorf("amount: %s", err) | |
| } | |
| var amount sdk.Coins | |
| if strings.TrimSpace(amountStr) != "" { | |
| amount, err = sdk.ParseCoinsNormalized(amountStr) | |
| if err != nil { | |
| return fmt.Errorf("amount: %s", err) | |
| } | |
| } | |
| label, err := cmd.Flags().GetString(cflags.FlagLabel) | |
| if err != nil { | |
| return fmt.Errorf("label: %s", err) | |
| } | |
| if label == "" { | |
| return errors.New("label is required on all contracts") | |
| } | |
| adminStr, err := cmd.Flags().GetString(cflags.FlagAdmin) | |
| if err != nil { | |
| return fmt.Errorf("admin: %s", err) | |
| } | |
| noAdmin, err := cmd.Flags().GetBool(cflags.FlagNoAdmin) | |
| if err != nil { | |
| return fmt.Errorf("no-admin: %s", err) | |
| } | |
| // ensure sensible admin is set (or explicitly immutable) | |
| if adminStr == "" && !noAdmin { | |
| return errors.New("you must set an admin or explicitly pass --no-admin to make it immutable (wasmd issue #719)") | |
| } | |
| if adminStr != "" && noAdmin { | |
| return errors.New("you set an admin and passed --no-admin, those cannot both be true") | |
| } | |
| if adminStr != "" { | |
| addr, err := sdk.AccAddressFromBech32(adminStr) | |
| if err != nil { | |
| info, err := clientCtx.Keyring.Key(adminStr) | |
| if err != nil { | |
| return fmt.Errorf("admin %s", err) | |
| } | |
| admin, err := info.GetAddress() | |
| if err != nil { | |
| return err | |
| } | |
| adminStr = admin.String() | |
| } else { | |
| adminStr = addr.String() | |
| } | |
| } | |
| storeAndInstantiateMsg := wtypes.MsgStoreAndInstantiateContract{ | |
| Authority: authority, | |
| WASMByteCode: storeCodeMsg.WASMByteCode, | |
| InstantiatePermission: storeCodeMsg.InstantiatePermission, | |
| UnpinCode: unpinCode, | |
| Source: source, | |
| Builder: builder, | |
| CodeHash: codeHash, | |
| Admin: adminStr, | |
| Label: label, | |
| Msg: []byte(args[1]), | |
| Funds: amount, | |
| } |
🤖 Prompt for AI Agents
In go/cli/gov_tx.go around lines 818-879, the code always calls
sdk.ParseCoinsNormalized on amountStr which fails for the default empty string;
change the flow to first check if the amount flag was actually provided
(cmd.Flags().Changed(cflags.FlagAmount)); only call sdk.ParseCoinsNormalized
when the flag was set and keep Funds nil when it wasn't; preserve existing error
handling for GetString and parsing, and set storeAndInstantiateMsg.Funds to the
parsed coins only when parsing occurred.
| amountStr, err := cmd.Flags().GetString(cflags.FlagAmount) | ||
| if err != nil { | ||
| return fmt.Errorf("amount: %s", err) | ||
| } | ||
| funds, err := sdk.ParseCoinsNormalized(amountStr) | ||
| if err != nil { | ||
| return fmt.Errorf("amount: %s", err) | ||
| } | ||
|
|
||
| msg := wtypes.MsgExecuteContract{ | ||
| Sender: authority, | ||
| Contract: contract, | ||
| Msg: execMsg, | ||
| Funds: funds, | ||
| } |
There was a problem hiding this comment.
Make --amount optional for execute proposal
Same issue here: parsing the empty default raises invalid decimal coin expression: "", so executions without funds cannot be submitted. Only parse when a value is provided and keep funds zero otherwise.
Apply this diff:
amountStr, err := cmd.Flags().GetString(cflags.FlagAmount)
if err != nil {
return fmt.Errorf("amount: %s", err)
}
- funds, err := sdk.ParseCoinsNormalized(amountStr)
- if err != nil {
- return fmt.Errorf("amount: %s", err)
- }
+ var funds sdk.Coins
+ if strings.TrimSpace(amountStr) != "" {
+ funds, err = sdk.ParseCoinsNormalized(amountStr)
+ if err != nil {
+ return fmt.Errorf("amount: %s", err)
+ }
+ }📝 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.
| amountStr, err := cmd.Flags().GetString(cflags.FlagAmount) | |
| if err != nil { | |
| return fmt.Errorf("amount: %s", err) | |
| } | |
| funds, err := sdk.ParseCoinsNormalized(amountStr) | |
| if err != nil { | |
| return fmt.Errorf("amount: %s", err) | |
| } | |
| msg := wtypes.MsgExecuteContract{ | |
| Sender: authority, | |
| Contract: contract, | |
| Msg: execMsg, | |
| Funds: funds, | |
| } | |
| amountStr, err := cmd.Flags().GetString(cflags.FlagAmount) | |
| if err != nil { | |
| return fmt.Errorf("amount: %s", err) | |
| } | |
| var funds sdk.Coins | |
| if strings.TrimSpace(amountStr) != "" { | |
| funds, err = sdk.ParseCoinsNormalized(amountStr) | |
| if err != nil { | |
| return fmt.Errorf("amount: %s", err) | |
| } | |
| } | |
| msg := wtypes.MsgExecuteContract{ | |
| Sender: authority, | |
| Contract: contract, | |
| Msg: execMsg, | |
| Funds: funds, | |
| } |
🤖 Prompt for AI Agents
In go/cli/gov_tx.go around lines 969 to 983, the code always attempts to parse
the --amount flag which fails on an empty default; change it to only call
sdk.ParseCoinsNormalized when amountStr is not the empty string and otherwise
leave funds as an empty/zero sdk.Coins (or nil) so the MsgExecuteContract can be
constructed with no funds; keep the existing error handling for
ParseCoinsNormalized and ensure funds is declared before the conditional so it
is available when building the msg.
| rm -f version.txt | ||
| echo "$tag" >version.tx |
There was a problem hiding this comment.
Fix typo in version file name.
After deleting version.txt, the script writes the tag into version.tx, leaving version.txt missing and breaking consumers that rely on it. Please write back to version.txt.
-rm -f version.txt
-echo "$tag" >version.tx
+rm -f version.txt
+echo "$tag" >version.txt📝 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.
| rm -f version.txt | |
| echo "$tag" >version.tx | |
| rm -f version.txt | |
| echo "$tag" >version.txt |
🤖 Prompt for AI Agents
In go/cli/testdata/wasm/download_releases.sh around lines 22 to 23, the script
removes version.txt but then writes the tag to version.tx due to a typo; change
the echo target to version.txt (i.e., echo "$tag" >version.txt) so the file
deleted by rm -f is recreated with the correct name.
| decoder := newArgDecoder(hex.DecodeString) | ||
| cmd := &cobra.Command{ | ||
| Use: "instantiate2 [code_id_int64] [json_encoded_init_args] [salt] --label [text] --admin [address,optional] --amount [coins,optional] " + | ||
| "--fix-msg [bool,optional]", | ||
| Short: "Instantiate a wasm contract with predictable address", | ||
| Long: fmt.Sprintf(`Creates a new instance of an uploaded wasm code with the given 'constructor' message. | ||
| Each contract instance has a unique address assigned. They are assigned automatically but in order to have predictable addresses | ||
| for special use cases, the given 'salt' argument and '--fix-msg' parameters can be used to generate a custom address. | ||
| Predictable address example (also see '%s query wasm build-address -h'): | ||
| $ %s tx wasm instantiate2 1 '{"foo":"bar"}' $(echo -n "testing" | xxd -ps) --admin="$(%s keys show mykey -a)" \ | ||
| --from mykey --amount="100ustake" --label "local0.1.0" \ | ||
| --fix-msg | ||
| `, version.AppName, version.AppName, version.AppName), | ||
| Aliases: []string{"start", "init", "inst", "i"}, | ||
| Args: cobra.ExactArgs(3), | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| ctx := cmd.Context() | ||
| cl := MustClientFromContext(ctx) | ||
| cctx := cl.ClientContext() | ||
|
|
||
| salt, err := decoder.DecodeString(args[2]) | ||
| if err != nil { | ||
| return fmt.Errorf("salt: %w", err) | ||
| } | ||
| fixMsg, err := cmd.Flags().GetBool(cflags.FlagFixMsg) | ||
| if err != nil { | ||
| return fmt.Errorf("fix msg: %w", err) | ||
| } | ||
| data, err := parseWasmInstantiateArgs(args[0], args[1], cctx.Keyring, cctx.GetFromAddress().String(), cmd.Flags()) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| msg := &types.MsgInstantiateContract2{ | ||
| Sender: data.Sender, | ||
| Admin: data.Admin, | ||
| CodeID: data.CodeID, | ||
| Label: data.Label, | ||
| Msg: data.Msg, | ||
| Funds: data.Funds, | ||
| Salt: salt, | ||
| FixMsg: fixMsg, | ||
| } | ||
|
|
||
| resp, err := cl.Tx().BroadcastMsgs(ctx, []sdk.Msg{msg}) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| return cl.PrintMessage(resp) | ||
| }, | ||
| SilenceUsage: true, | ||
| } | ||
|
|
||
| cmd.Flags().String(cflags.FlagAmount, "", "Coins to send to the contract during instantiation") | ||
| cmd.Flags().String(cflags.FlagLabel, "", "A human-readable name for this contract in lists") | ||
| cmd.Flags().String(cflags.FlagAdmin, "", "Address or key name of an admin") | ||
| cmd.Flags().Bool(cflags.FlagNoAdmin, false, "You must set this explicitly if you don't want an admin") | ||
| cmd.Flags().Bool(cflags.FlagFixMsg, false, "An optional flag to include the json_encoded_init_args for the predictable address generation mode") | ||
| decoder.RegisterFlags(cmd.PersistentFlags(), "salt") | ||
| cflags.AddTxFlagsToCmd(cmd) | ||
| return cmd | ||
| } |
There was a problem hiding this comment.
Add TxPersistentPreRunE to instantiate2 command
This command is the only wasm tx helper that skips TxPersistentPreRunE. As a result the client never gets injected into the context before MustClientFromContext runs, so akash tx wasm instantiate2 will panic instead of broadcasting. Please wire the pre-run hook just like the other tx commands.
Apply this diff:
func GetTxWasmInstantiateContract2Cmd() *cobra.Command {
decoder := newArgDecoder(hex.DecodeString)
cmd := &cobra.Command{
Use: "instantiate2 [code_id_int64] [json_encoded_init_args] [salt] --label [text] --admin [address,optional] --amount [coins,optional] " +
"--fix-msg [bool,optional]",
Short: "Instantiate a wasm contract with predictable address",
Aliases: []string{"start", "init", "inst", "i"},
- Args: cobra.ExactArgs(3),
+ Args: cobra.ExactArgs(3),
+ PersistentPreRunE: TxPersistentPreRunE,
RunE: func(cmd *cobra.Command, args []string) error {🤖 Prompt for AI Agents
In go/cli/wasm_tx.go around lines 129-191, the instantiate2 command never wires
the TxPersistentPreRunE hook so the client isn't injected before
MustClientFromContext is called; set the command's PersistentPreRunE to the
shared TxPersistentPreRunE used by other tx commands (e.g. cmd.PersistentPreRunE
= client.TxPersistentPreRunE or the package-level TxPersistentPreRunE your
codebase uses) so the client context is prepared before RunE executes.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
go/sdl/go.mod (1)
23-23: Confirm that the akash fork of cosmos-sdk is actively maintained.The replace directive uses a fork maintained by akash-network. Ensure that:
- The fork is actively maintained and tracks upstream cosmos-sdk security patches
- The "-akash.8" suffix indicates the fork version relative to the upstream (v0.53.4)
- There's a process in place to merge critical upstream fixes back into the fork
If you need help tracking the fork status or setting up automated monitoring for upstream patches, I can help.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
go/cli/go.sumis excluded by!**/*.sumgo/go.sumis excluded by!**/*.sumgo/sdl/go.sumis excluded by!**/*.sum
📒 Files selected for processing (3)
go/cli/go.mod(12 hunks)go/go.mod(9 hunks)go/sdl/go.mod(8 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: coverage
- GitHub Check: test
- GitHub Check: breakage
- GitHub Check: proto
- GitHub Check: go
🔇 Additional comments (7)
go/sdl/go.mod (3)
10-11: Direct dependency versions and fork alignment look consistent.The cosmos-sdk direct dependency upgrade to v0.53.4 is aligned with the akash fork replacement (v0.53.4-akash.8 at line 23). The testify patch upgrade to v1.11.1 appears safe and routine.
Also applies to: 23-23
174-186: I have the protobuf confirmation. Let me verify the golang.org/x packages as well since the original review comment specifically asks about "Multiple golang.org/x packages."Based on my verification:
All dependency updates are compatible with cosmos-sdk v0.53.4.
The cosmos-sdk v0.53.4 release is fully API and state-compatible with all v0.53.x releases and is a patch update that does not require a chain upgrade.
Specifically:
- The v0.53.4 patch release addresses a x/distribution module issue with only minimal, non-breaking changes.
- The cosmos-sdk v0.53.4 is fully API and state-compatible with all v0.53.x releases. The protobuf v1.36.8 bump has been confirmed compatible with v0.53.4 (non-breaking client library release).
- The golang.org/x packages you've updated (crypto, exp, net, sync, sys, term, text) are external dependencies managed by the Go module system and are safe for v0.53.4, which uses modern Go versions that include native standard library equivalents for packages previously sourced from golang.org/x.
1-199: Verify all indirect dependencies and CI pipeline stability locally.The go.mod file is syntactically valid and all modules resolve correctly. Spot checks confirm no known CVEs in golang.org/x/crypto v0.41.0 and cosmos-sdk v0.53.4 is a stable minor patch. However, sandbox testing cannot fully verify:
- Whether all indirect dependency updates cascaded from cosmos-sdk v0.53.3→v0.53.4 and testify v1.10.0→v1.11.1 upgrades, or represent intentional lockfile maintenance
- Full govulncheck scan of the entire dependency tree
- CI pipeline stability and build success
Ensure locally via
go mod tidy,govulncheck ./..., and your CI pipeline that:
- No unexpected transitive dependencies entered the graph
- All dependency versions pass vulnerability checks
- The build succeeds end-to-end
go/cli/go.mod (2)
20-21: Direct wasmvm/v3 dependency in CLI is necessary and justified.The CLI module directly imports wasmvm types in multiple files (wasm_query.go, wasm_parse.go, gov_tx.go, and testdata contracts.go), confirming that the direct dependency on wasmvm/v3 v3.0.2 in go.mod is correct and necessary. The concern about managing it transitively through wasmd is unfounded—the CLI operations require direct access to wasmvm types.
Likely an incorrect or invalid review comment.
89-89: Verify if Ethereum dependencies from wasmd/ibc-go are needed for your CLI use case.The review comment is accurate. The
go mod graphshowsethereum/go-ethereum v1.15.11is pulled in transitively through two paths:
- github.com/CosmWasm/wasmd v0.61.6 requires github.com/ethereum/go-ethereum
- github.com/cosmos/ibc-go/v10 requires github.com/ethereum/go-ethereum
These are legitimate Cosmos ecosystem dependencies—go-ethereum is typically required when Cosmos modules support Ethereum/EVM compatibility features. However, since no Ethereum imports exist in your CLI code, these dependencies are unnecessary if your CLI doesn't require EVM support.
Action: Verify whether your CLI use case requires EVM compatibility. If not, you can add replace directives in
go/cli/go.modto point to alternative implementations (e.g., Cosmos-forked versions) or evaluate if a lighter dependency tree is possible with updated package versions.go/go.mod (2)
10-10: No breaking changes documented between Cosmos SDK v0.53.4 and protobuf v1.36.8.There are no widely reported, specific incompatibilities between Cosmos SDK v0.53.4 and google.golang.org/protobuf v1.36.8. The usual protobuf-version rules apply: use matching generator/runtime versions and the SDK's recommended proto toolchain. The patch bump from v0.53.3→v0.53.4 combined with protobuf v1.36.8 does not introduce the breaking changes flagged in the original comment. No action required.
17-17: wasmd v0.61.6 and wasmvm/v3 v3.0.2 are compatible and secure.wasmd v0.61.6 depends on cosmos-sdk v0.53.4, which aligns with the PR's cosmos-sdk v0.53.4-akash.8 fork. CWA-2024-006 (the non-deterministic module_query_safe query issue) was fixed in wasmd v0.53.0, so v0.61.6 includes all necessary security patches. wasmvm v3.0.2 has no public advisories—known vulnerabilities in the wasmvm project target v1.x and v2.x version lines only.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
go/cli/root.go (1)
33-48: Clarify the exported PreRunOption API surface.Exporting
PreRunOptionimplies downstream code can craft custom options, but the only mutable state lives in unexported fields (appConfigTemplate,appConfig,cmtCfg) onPreRunOptions. Callers in other packages can’t touch those fields, so their custom option closures can’t do anything useful. Either keep the option type internal (lower-case) or expose setters/accessors so external callers can extend the option set without resorting to reflection.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
go/cli/configs.go(7 hunks)go/cli/flags/flags.go(1 hunks)go/cli/root.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
go/cli/configs.go (1)
go/cli/root.go (2)
PreRunOption(31-31)PreRunOptions(25-29)
go/cli/root.go (1)
go/cli/configs.go (1)
InterceptConfigsPreRunHandler(149-252)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: coverage
- GitHub Check: test
- GitHub Check: breakage
- GitHub Check: proto
- GitHub Check: go
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
go/cli/genesis_init.go (1)
16-24: Resolve duplicate cometbft config import.Importing
github.com/cometbft/cometbft/configtwice (ascmtcfgandcfg) fails to compile. Reuse the existingcfgalias instead.Apply this diff to fix it:
-import ( - "bufio" - "encoding/json" - "errors" - "fmt" - "io" - "net/http" - "os" - "path/filepath" - "time" - - "github.com/spf13/cobra" - - cmtcfg "github.com/cometbft/cometbft/config" +import ( + "bufio" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "time" + + "github.com/spf13/cobra"-func downloadGenesis(config *cmtcfg.Config, chainID string) error { +func downloadGenesis(config *cfg.Config, chainID string) error {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
go/cli/genesis_init.go(4 hunks)go/cli/module_query.go(1 hunks)go/cli/query.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- go/cli/query.go
🧰 Additional context used
🧬 Code graph analysis (2)
go/cli/module_query.go (2)
go/cli/cctx.go (1)
GetClientContextFromCmd(36-47)go/cli/flags/flags.go (2)
AddPaginationFlagsToCmd(389-396)AddQueryFlagsToCmd(330-340)
go/cli/genesis_init.go (1)
go/cli/flags/flags.go (3)
FlagChainID(100-100)FlagDefaultBondDenom(51-51)FlagConsensusKeyAlgo(53-53)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: shell
- GitHub Check: breakage
- GitHub Check: test
- GitHub Check: coverage
- GitHub Check: proto
- GitHub Check: go
🔇 Additional comments (1)
go/cli/module_query.go (1)
1-9: LGTM!The imports are appropriate for a Cosmos SDK CLI query command.
| genesisURL := fmt.Sprintf("https://github.com/osmosis-labs/osmosis/raw/main/networks/%s/genesis.json?download", chainID) | ||
|
|
||
| // Determine the destination path for the genesis file | ||
| genFilePath := config.GenesisFile() | ||
|
|
||
| // Create a new HTTP client with a 30-second timeout | ||
| client := &http.Client{ | ||
| Timeout: 30 * time.Second, | ||
| } | ||
|
|
||
| // Create a new GET request | ||
| req, err := http.NewRequest("GET", genesisURL, nil) | ||
| if err != nil { | ||
| return errorsmod.Wrap(err, "failed to create HTTP request for genesis file") | ||
| } | ||
|
|
||
| // Send the request | ||
| fmt.Println("Attempting to download genesis file from", genesisURL) | ||
| resp, err := client.Do(req) | ||
| if err != nil { | ||
| return errorsmod.Wrap(err, "failed to download genesis file") | ||
| } | ||
| defer func() { | ||
| _ = resp.Body.Close() | ||
| }() | ||
|
|
||
| // Check if the HTTP request was successful | ||
| if resp.StatusCode != http.StatusOK { | ||
| return fmt.Errorf("failed to download genesis file: HTTP status %d", resp.StatusCode) | ||
| } |
There was a problem hiding this comment.
Point the genesis download at the Akash source.
The helper is documented as fetching from the Akash Network repo, but the URL targets osmosis-labs/osmosis. That will never yield the Akash genesis (and likely 404s), so the download path can’t work for mainnet users. Update the base URL to the actual Akash genesis location (e.g. the akash-network repo/path used today) before landing this change.
🤖 Prompt for AI Agents
In go/cli/genesis_init.go around lines 215 to 244, the genesis download URL
currently points to the osmosis repo which is incorrect for Akash; update the
base URL to the Akash genesis location (replace
"https://github.com/osmosis-labs/osmosis/raw/main/networks/%s/genesis.json?download"
with the akash repo path, e.g.
"https://github.com/akash-network/akash/raw/main/networks/%s/genesis.json?download"),
ensuring chainID is still interpolated, so the helper fetches the correct Akash
genesis file and returns proper errors on non-200 responses.
| func GetQueryModuleNameToAddressCmd() *cobra.Command { | ||
| cmd := &cobra.Command{ | ||
| Use: "module-name-to-address [module-name]", | ||
| Short: "module name to address", | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| clientCtx := client.GetClientContextFromCmd(cmd) | ||
| address := authtypes.NewModuleAddress(args[0]) | ||
| return clientCtx.PrintString(address.String()) | ||
| }, | ||
| } |
There was a problem hiding this comment.
Add argument validation to prevent runtime panic.
The command accesses args[0] without validating that an argument was provided, which will cause a panic if the command is invoked without arguments.
Apply this diff to add argument validation:
func GetQueryModuleNameToAddressCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "module-name-to-address [module-name]",
Short: "module name to address",
+ Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
clientCtx := client.GetClientContextFromCmd(cmd)
address := authtypes.NewModuleAddress(args[0])
return clientCtx.PrintString(address.String())
},
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func GetQueryModuleNameToAddressCmd() *cobra.Command { | |
| cmd := &cobra.Command{ | |
| Use: "module-name-to-address [module-name]", | |
| Short: "module name to address", | |
| RunE: func(cmd *cobra.Command, args []string) error { | |
| clientCtx := client.GetClientContextFromCmd(cmd) | |
| address := authtypes.NewModuleAddress(args[0]) | |
| return clientCtx.PrintString(address.String()) | |
| }, | |
| } | |
| func GetQueryModuleNameToAddressCmd() *cobra.Command { | |
| cmd := &cobra.Command{ | |
| Use: "module-name-to-address [module-name]", | |
| Short: "module name to address", | |
| Args: cobra.ExactArgs(1), | |
| RunE: func(cmd *cobra.Command, args []string) error { | |
| clientCtx := client.GetClientContextFromCmd(cmd) | |
| address := authtypes.NewModuleAddress(args[0]) | |
| return clientCtx.PrintString(address.String()) | |
| }, | |
| } |
🤖 Prompt for AI Agents
In go/cli/module_query.go around lines 11 to 20, the command reads args[0]
without checking arguments which will panic if none are provided; enforce
argument validation by adding an Args validator (e.g., cobra.ExactArgs(1)) to
the command definition or explicitly check len(args) in RunE and return a
descriptive error when no argument is supplied, then only access args[0] after
validation so the command fails gracefully instead of panicking.
| }, | ||
| } | ||
|
|
||
| cflags.AddPaginationFlagsToCmd(cmd, cmd.Use) |
There was a problem hiding this comment.
Remove unnecessary pagination flags.
This command returns a single module address, not a paginated list. Pagination flags (page, page-key, offset, limit, count-total, reverse) don't apply and will confuse users.
Apply this diff to remove the pagination flags:
- cflags.AddPaginationFlagsToCmd(cmd, cmd.Use)
cflags.AddQueryFlagsToCmd(cmd)📝 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.
| cflags.AddPaginationFlagsToCmd(cmd, cmd.Use) | |
| cflags.AddQueryFlagsToCmd(cmd) |
🤖 Prompt for AI Agents
In go/cli/module_query.go at line 22, pagination flags are being added with
cflags.AddPaginationFlagsToCmd(cmd, cmd.Use) even though this command returns a
single module address; remove that call so the pagination flags (page, page-key,
offset, limit, count-total, reverse) are not registered for this command to
avoid confusing users. Simply delete or comment out the
cflags.AddPaginationFlagsToCmd(cmd, cmd.Use) line and ensure no other code
depends on those flags for this command.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (14)
proto/node/akash/wasm/v1/service.proto (1)
10-10: Fix copy-paste error in comment.The comment references "market Msg service" but this is the wasm module service.
Apply this diff:
-// Msg defines the market Msg service +// Msg defines the wasm Msg servicego/cli/module_query.go (2)
11-20: Add argument validation to prevent runtime panic.The command accesses
args[0]without validating that an argument was provided, which will cause a panic if invoked without arguments.Apply this diff to add argument validation:
func GetQueryModuleNameToAddressCmd() *cobra.Command { cmd := &cobra.Command{ Use: "module-name-to-address [module-name]", Short: "module name to address", + Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { clientCtx := client.GetClientContextFromCmd(cmd) address := authtypes.NewModuleAddress(args[0]) return clientCtx.PrintString(address.String()) }, }
22-22: Remove unnecessary pagination flags.This command returns a single module address, not a paginated list. Pagination flags don't apply and will confuse users.
Apply this diff to remove the pagination flags:
- cflags.AddPaginationFlagsToCmd(cmd, cmd.Use) cflags.AddQueryFlagsToCmd(cmd)proto/node/akash/wasm/v1/query.proto (1)
24-26: Fix copy-paste errors in comments.The comments reference "take package" and "minting parameters" but this is the wasm module.
Apply this diff:
-// Query defines the gRPC querier service of the take package. +// Query defines the gRPC querier service of the wasm module. service Query { - // Params returns the total set of minting parameters. + // Params returns the total set of wasm parameters. rpc Params(QueryParamsRequest) returns (QueryParamsResponse) {go/node/wasm/v1/params.go (3)
23-25: Add validation for blocked_addresses.The
Validatemethod performs no checks on theBlockedAddressesfield. Consider validating that each address in the list is a valid Bech32 address format to prevent invalid configuration.Apply this diff to add validation:
+import ( + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "cosmossdk.io/errors" +) + func (p Params) Validate() error { + for _, addr := range p.BlockedAddresses { + if _, err := sdk.AccAddressFromBech32(addr); err != nil { + return sdkerrors.Wrapf(err, "invalid blocked address: %s", addr) + } + } return nil }
9-10: Fix the copy-paste errors in comments.The comments reference "take module" and "take store" but this is the wasm module.
Apply this diff:
-// ParamKeyTable for take module -// Deprecated: now params can be accessed on key `0x01` on the take store. +// ParamKeyTable for wasm module +// Deprecated: now params can be accessed on key `0x01` on the wasm store.
15-17: Register BlockedAddresses in ParamSetPairs.The
Paramsstruct contains aBlockedAddressesfield (visible in params.pb.go), butParamSetPairs()returns an empty set. Other modules in the codebase register their fields here with validation. Either registerBlockedAddresseswith a validator function, or clarify why it's intentionally excluded.proto/node/akash/wasm/v1/paramsmsg.proto (1)
24-24: Fix the copy-paste error in the comment.The comment incorrectly references "x/deployment parameters" but this message is for the x/wasm module.
Apply this diff:
- // params defines the x/deployment parameters to update. + // params defines the x/wasm parameters to update.go/node/wasm/v1/msgs.go (1)
28-31: Handle Bech32 decode failure in GetSigners.
GetSignerssilently drops the error fromsdk.AccAddressFromBech32, returning a nil signer whenAuthorityis malformed. Since BaseApp collects signers beforeValidateBasic, an invalid address will slip through until signature verification, causing non-deterministic panics or nil dereferences.Apply this diff to fail explicitly:
func (m *MsgUpdateParams) GetSigners() []sdk.AccAddress { - addr, _ := sdk.AccAddressFromBech32(m.Authority) - return []sdk.AccAddress{addr} + addr, err := sdk.AccAddressFromBech32(m.Authority) + if err != nil { + panic(err) + } + return []sdk.AccAddress{addr} }go/cli/genesis_init.go (2)
129-190: Persist config even when download succeeds.When
downloadGenesissucceeds at line 134, the function returns at line 189 without writingconfig.toml(line 185), configuring consensus keys (lines 172-177), or printing init info (line 186). This breaks theinitcommand's contract.Restructure so the finalization block (lines 172-186) executes regardless of whether genesis was downloaded or generated. Only the genesis generation itself (lines 143-180) should be conditional.
213-259: Point genesis download at the Akash source.The URL at line 215 targets
osmosis-labs/osmosis, not the Akash Network. This will fail (likely 404) for Akash mainnet users.Update the base URL to the actual Akash genesis location:
- genesisURL := fmt.Sprintf("https://github.com/osmosis-labs/osmosis/raw/main/networks/%s/genesis.json?download", chainID) + genesisURL := fmt.Sprintf("https://github.com/akash-network/akash/raw/main/networks/%s/genesis.json?download", chainID)go/cli/wasm_tx.go (1)
128-191: Missing PersistentPreRunE causes runtime panic.The
instantiate2command is missingPersistentPreRunE: TxPersistentPreRunE, so the client never gets injected into the context beforeMustClientFromContextexecutes on line 147. This will cause a panic instead of broadcasting the transaction.Apply this diff:
func GetTxWasmInstantiateContract2Cmd() *cobra.Command { decoder := newArgDecoder(hex.DecodeString) cmd := &cobra.Command{ Use: "instantiate2 [code_id_int64] [json_encoded_init_args] [salt] --label [text] --admin [address,optional] --amount [coins,optional] " + "--fix-msg [bool,optional]", Short: "Instantiate a wasm contract with predictable address", - Aliases: []string{"start", "init", "inst", "i"}, - Args: cobra.ExactArgs(3), + Aliases: []string{"start", "init", "inst", "i"}, + Args: cobra.ExactArgs(3), + PersistentPreRunE: TxPersistentPreRunE, RunE: func(cmd *cobra.Command, args []string) error {go/cli/gov_tx.go (2)
818-825: Optional amount flag fails on empty string.
sdk.ParseCoinsNormalizedrejects empty strings withinvalid decimal coin expression: "", so users cannot submit proposals without--amounteven though the help text marks it optional.Apply this diff:
amountStr, err := cmd.Flags().GetString(cflags.FlagAmount) if err != nil { return fmt.Errorf("amount: %s", err) } - amount, err := sdk.ParseCoinsNormalized(amountStr) - if err != nil { - return fmt.Errorf("amount: %s", err) - } + var amount sdk.Coins + if strings.TrimSpace(amountStr) != "" { + amount, err = sdk.ParseCoinsNormalized(amountStr) + if err != nil { + return fmt.Errorf("amount: %s", err) + } + }
969-976: Optional amount flag fails on empty string.Same issue as the store-instantiate proposal: parsing always fails when
--amountis omitted, preventing execution proposals without funds.Apply this diff:
amountStr, err := cmd.Flags().GetString(cflags.FlagAmount) if err != nil { return fmt.Errorf("amount: %s", err) } - funds, err := sdk.ParseCoinsNormalized(amountStr) - if err != nil { - return fmt.Errorf("amount: %s", err) - } + var funds sdk.Coins + if strings.TrimSpace(amountStr) != "" { + funds, err = sdk.ParseCoinsNormalized(amountStr) + if err != nil { + return fmt.Errorf("amount: %s", err) + } + }
🧹 Nitpick comments (1)
go/cli/configs.go (1)
149-166: Nice refactor to options pattern!The migration from fixed parameters to variadic
PreRunOptionimproves extensibility and aligns with Go best practices for optional configuration.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (22)
go/cli/go.sumis excluded by!**/*.sumgo/cli/testdata/wasm/burner.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/cyberpunk.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/hackatom.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/hackatom_42.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/hackatom_420.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/ibc_reflect.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/queue.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/reflect_1_1.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/reflect_1_5.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/reflect_2_0.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/replier.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/staking.wasmis excluded by!**/*.wasmgo/go.sumis excluded by!**/*.sumgo/node/wasm/v1/event.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/genesis.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/params.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/paramsmsg.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/query.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/query.pb.gw.gois excluded by!**/*.pb.gw.gogo/node/wasm/v1/service.pb.gois excluded by!**/*.pb.gogo/sdl/go.sumis excluded by!**/*.sum
📒 Files selected for processing (31)
buf.yaml(1 hunks)go/cli/authz_tx.go(2 hunks)go/cli/configs.go(7 hunks)go/cli/flags/flags.go(1 hunks)go/cli/genesis_init.go(4 hunks)go/cli/go.mod(11 hunks)go/cli/gov_tx.go(5 hunks)go/cli/module_query.go(1 hunks)go/cli/query.go(1 hunks)go/cli/root.go(1 hunks)go/cli/testdata/wasm/contracts.go(1 hunks)go/cli/testdata/wasm/download_releases.sh(1 hunks)go/cli/tx.go(1 hunks)go/cli/wasm_parse.go(1 hunks)go/cli/wasm_query.go(1 hunks)go/cli/wasm_tx.go(1 hunks)go/cli/wasm_tx_test.go(1 hunks)go/go.mod(8 hunks)go/node/client/v1beta3/client.go(2 hunks)go/node/client/v1beta3/query.go(4 hunks)go/node/wasm/v1/codec.go(1 hunks)go/node/wasm/v1/key.go(1 hunks)go/node/wasm/v1/msgs.go(1 hunks)go/node/wasm/v1/params.go(1 hunks)go/sdl/go.mod(7 hunks)proto/node/akash/wasm/v1/event.proto(1 hunks)proto/node/akash/wasm/v1/genesis.proto(1 hunks)proto/node/akash/wasm/v1/params.proto(1 hunks)proto/node/akash/wasm/v1/paramsmsg.proto(1 hunks)proto/node/akash/wasm/v1/query.proto(1 hunks)proto/node/akash/wasm/v1/service.proto(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- go/node/wasm/v1/codec.go
🚧 Files skipped from review as they are similar to previous changes (7)
- go/node/client/v1beta3/client.go
- go/go.mod
- go/node/wasm/v1/key.go
- go/cli/wasm_tx_test.go
- go/cli/tx.go
- go/cli/testdata/wasm/download_releases.sh
- buf.yaml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-16T10:32:01.602Z
Learnt from: cloud-j-luna
Repo: akash-network/chain-sdk PR: 118
File: go/cli/tx.go:48-63
Timestamp: 2025-10-16T10:32:01.602Z
Learning: In the Akash chain-sdk CLI (Go), when `cctx.Offline` is true, nil clients are intentionally stored in the context using `ContextTypeClient` and `ContextTypeQueryClient` keys. Operations that support offline mode should check for `ErrContextValueNotSet` when retrieving clients from context using `ClientFromContext()` or `QueryClientFromContext()`, rather than using the `Must*` variants which panic on nil values.
Applied to files:
go/node/client/v1beta3/query.go
🧬 Code graph analysis (13)
go/cli/genesis_init.go (1)
go/cli/flags/flags.go (3)
FlagChainID(100-100)FlagDefaultBondDenom(51-51)FlagConsensusKeyAlgo(53-53)
go/node/client/v1beta3/query.go (1)
go/node/client/v1beta3/client.go (1)
QueryClient(40-63)
go/cli/query.go (2)
go/cli/wasm_query.go (1)
GetQueryWasmCmd(25-49)go/cli/module_query.go (1)
GetQueryModuleNameToAddressCmd(11-26)
go/node/wasm/v1/params.go (1)
go/node/wasm/v1/params.pb.go (3)
Params(28-30)Params(34-34)Params(35-37)
go/cli/module_query.go (2)
go/cli/cctx.go (1)
GetClientContextFromCmd(36-47)go/cli/flags/flags.go (2)
AddPaginationFlagsToCmd(389-396)AddQueryFlagsToCmd(330-340)
go/cli/wasm_tx.go (3)
go/cli/tx.go (1)
TxPersistentPreRunE(13-52)go/cli/context.go (1)
MustClientFromContext(47-54)go/cli/flags/flags.go (10)
AddTxFlagsToCmd(343-380)FlagInstantiateByEverybody(270-270)FlagInstantiateNobody(271-271)FlagInstantiateByAddress(272-272)FlagInstantiateByAnyOfAddress(273-273)FlagAmount(163-163)FlagLabel(262-262)FlagAdmin(266-266)FlagNoAdmin(267-267)FlagFixMsg(268-268)
go/node/wasm/v1/msgs.go (4)
go/node/wasm/v1/paramsmsg.pb.go (3)
MsgUpdateParams(31-38)MsgUpdateParams(42-42)MsgUpdateParams(43-45)go/node/wasm/v1/params.pb.go (3)
Params(28-30)Params(34-34)Params(35-37)go/node/wasm/v1/codec.go (1)
ModuleCdc(19-19)go/node/wasm/v1/key.go (1)
RouterKey(11-11)
go/cli/configs.go (1)
go/cli/root.go (2)
PreRunOption(31-31)PreRunOptions(25-29)
go/cli/authz_tx.go (2)
go/cli/context.go (1)
MustClientFromContext(47-54)go/cli/flags/flags.go (8)
FlagAllowedMsgKeys(275-275)FlagAllowedRawMsgs(276-276)FlagMaxFunds(278-278)FlagMaxCalls(277-277)FlagExpiration(94-94)FlagAllowAllMsgs(279-279)FlagNoTokenTransfer(280-280)AddTxFlagsToCmd(343-380)
go/cli/root.go (1)
go/cli/configs.go (1)
InterceptConfigsPreRunHandler(149-252)
go/cli/gov_tx.go (2)
go/cli/flags/flags.go (15)
FlagAuthority(186-186)FlagAmount(163-163)FlagLabel(262-262)FlagAdmin(266-266)FlagNoAdmin(267-267)FlagFixMsg(268-268)FlagUnpinCode(274-274)FlagSource(263-263)FlagBuilder(264-264)FlagCodeHash(265-265)AddTxFlagsToCmd(343-380)FlagTitle(69-69)FlagSummary(71-71)FlagDeposit(92-92)FlagExpedite(281-281)go/cli/cctx.go (1)
GetClientTxContext(81-84)
go/cli/wasm_parse.go (1)
go/cli/flags/flags.go (11)
FlagSource(263-263)FlagBuilder(264-264)FlagCodeHash(265-265)FlagInstantiateByAnyOfAddress(273-273)FlagInstantiateByAddress(272-272)FlagInstantiateByEverybody(270-270)FlagInstantiateNobody(271-271)FlagAmount(163-163)FlagLabel(262-262)FlagAdmin(266-266)FlagNoAdmin(267-267)
go/cli/wasm_query.go (4)
go/cli/query.go (1)
QueryPersistentPreRunE(12-38)go/cli/context.go (1)
MustLightClientFromContext(75-82)go/cli/utils.go (1)
ReadPageRequest(193-225)go/cli/flags/flags.go (2)
AddQueryFlagsToCmd(330-340)AddPaginationFlagsToCmd(389-396)
🪛 Buf (1.59.0)
proto/node/akash/wasm/v1/event.proto
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/wasm/v1/genesis.proto
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/wasm/v1/params.proto
5-5: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/wasm/v1/paramsmsg.proto
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/wasm/v1/query.proto
5-5: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/wasm/v1/service.proto
4-4: import "cosmos/msg/v1/msg.proto": file does not exist
(COMPILE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: coverage
- GitHub Check: test
- GitHub Check: go
- GitHub Check: breakage
🔇 Additional comments (18)
go/node/client/v1beta3/query.go (4)
7-7: LGTM!The import follows the established aliased pattern and correctly brings in the wasm query types needed for the new functionality.
44-44: LGTM!The field addition is consistent with the existing struct design and properly typed to match the interface requirement.
85-85: LGTM!The initialization follows the exact same pattern as all other query clients in this function, maintaining consistency with the established architecture.
186-189: LGTM!The Wasm() accessor method correctly implements the QueryClient interface and follows the established pattern used by all other query client accessors in this file.
go/cli/testdata/wasm/contracts.go (1)
1-99: LGTM! Well-structured test data.The embedded WASM contract setup and message type definitions are clean and well-organized. The use of
//go:embedfor test contracts and the exported accessor functions provide a clear API for test scenarios.go/cli/query.go (1)
72-73: LGTM! Clean command integration.The new wasm and module query commands are properly integrated into the query command hierarchy.
proto/node/akash/wasm/v1/genesis.proto (1)
1-17: LGTM! Standard genesis state definition.The
GenesisStatemessage follows standard Cosmos SDK patterns with appropriate gogoproto options for the non-nullableParamsfield.proto/node/akash/wasm/v1/params.proto (1)
1-17: LGTM! Clean parameter definition.The
Paramsmessage withblocked_addressesfield is well-structured with appropriate cosmos proto annotations and serialization options.go/sdl/go.mod (1)
7-198: Dependency updates are secure and fix a HIGH severity vulnerability.The cosmos-sdk v0.53.4 update resolves a HIGH severity Integer Overflow vulnerability in the Validator Rewards pool that affects versions < 0.53.3, providing improved security. Testify v1.11.1 has no known security advisories.
proto/node/akash/wasm/v1/event.proto (1)
1-29: LGTM!The event message is well-structured with appropriate field annotations for address validation and serialization formats. The use of cosmos.AddressString scalar for contract_address ensures proper address validation.
go/cli/go.mod (1)
1-266: LGTM!The dependency updates appropriately add CosmWasm support (wasmd v0.61.6, wasmvm/v3 v3.0.2) and bump related packages. The version upgrades align with the wasm module integration across the PR.
go/cli/configs.go (1)
311-384: LGTM!The addition of the
confparameter with nil-check defaulting toDefaultConfig()maintains backward compatibility while enabling custom CometBFT configuration injection.go/cli/authz_tx.go (2)
421-473: No implementation concerns found—parseStoreCodeGrantsis properly implemented.The function exists in
go/cli/wasm_tx.go(line 225) and correctly parses thecode_hash:access_configformat as documented in the usage examples. It handles both wildcard (*) and specific permission cases, with appropriate error handling.Likely an incorrect or invalid review comment.
269-419: The verification request references an unrelated function.The
parseStoreCodeGrantsfunction does exist atgo/cli/wasm_tx.go:225, but it is not used or referenced in the code being reviewed inauthz_tx.go(GetTxAuthzGrantContractAuthorizationCmd). This function operates in a different context (store code grants vs. contract authorization grants) and is not relevant to the changes in this review.Likely an incorrect or invalid review comment.
go/cli/root.go (1)
25-48: LGTM! Clean functional options pattern.The new
PreRunOptionsand related functions provide a flexible, idiomatic way to configure pre-run behavior. The functional options pattern is well-implemented.go/cli/flags/flags.go (1)
262-289: LGTM! Well-organized flag constants.The new WASM-related flag constants are clearly defined and properly grouped. The naming is consistent with existing patterns.
go/cli/wasm_query.go (1)
599-640: LGTM! Flexible argument decoder implementation.The
argumentDecodertype provides a clean way to support multiple encoding formats (ascii, hex, base64) with mutual exclusion checks. The implementation follows good practices.go/cli/gov_tx.go (1)
68-70: LGTM! Clear extension point for governance authority.The
DefaultGovAuthorityvariable provides a well-documented extension point for chains to customize the governance module address. The comment clearly explains its purpose.
go/cli/wasm_parse.go
Outdated
| func parseWasmAccessConfig(raw string) (c types.AccessConfig, err error) { | ||
| switch raw { | ||
| case "nobody": | ||
| return types.AllowNobody, nil | ||
| case "everybody": | ||
| return types.AllowEverybody, nil | ||
| default: | ||
| parts := strings.Split(raw, ",") | ||
| addrs := make([]sdk.AccAddress, len(parts)) | ||
| for i, v := range parts { | ||
| addr, err := sdk.AccAddressFromBech32(v) | ||
| if err != nil { | ||
| return types.AccessConfig{}, fmt.Errorf("unable to parse address %q: %s", v, err) | ||
| } | ||
| addrs[i] = addr | ||
| } | ||
| defer func() { // convert panic in ".With" to error for better output | ||
| if r := recover(); r != nil { | ||
| err = r.(error) | ||
| } | ||
| }() | ||
| cfg := types.AccessTypeAnyOfAddresses.With(addrs...) | ||
| return cfg, cfg.ValidateBasic() | ||
| } | ||
| } |
There was a problem hiding this comment.
Unsafe type assertion in panic recovery.
The defer function assumes the recovered panic is an error type without verification. If .With() panics with a non-error value, this will cause a secondary panic.
Apply this diff to add a type check:
defer func() { // convert panic in ".With" to error for better output
if r := recover(); r != nil {
- err = r.(error)
+ var ok bool
+ err, ok = r.(error)
+ if !ok {
+ err = fmt.Errorf("unexpected panic: %v", r)
+ }
}
}()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func parseWasmAccessConfig(raw string) (c types.AccessConfig, err error) { | |
| switch raw { | |
| case "nobody": | |
| return types.AllowNobody, nil | |
| case "everybody": | |
| return types.AllowEverybody, nil | |
| default: | |
| parts := strings.Split(raw, ",") | |
| addrs := make([]sdk.AccAddress, len(parts)) | |
| for i, v := range parts { | |
| addr, err := sdk.AccAddressFromBech32(v) | |
| if err != nil { | |
| return types.AccessConfig{}, fmt.Errorf("unable to parse address %q: %s", v, err) | |
| } | |
| addrs[i] = addr | |
| } | |
| defer func() { // convert panic in ".With" to error for better output | |
| if r := recover(); r != nil { | |
| err = r.(error) | |
| } | |
| }() | |
| cfg := types.AccessTypeAnyOfAddresses.With(addrs...) | |
| return cfg, cfg.ValidateBasic() | |
| } | |
| } | |
| func parseWasmAccessConfig(raw string) (c types.AccessConfig, err error) { | |
| switch raw { | |
| case "nobody": | |
| return types.AllowNobody, nil | |
| case "everybody": | |
| return types.AllowEverybody, nil | |
| default: | |
| parts := strings.Split(raw, ",") | |
| addrs := make([]sdk.AccAddress, len(parts)) | |
| for i, v := range parts { | |
| addr, err := sdk.AccAddressFromBech32(v) | |
| if err != nil { | |
| return types.AccessConfig{}, fmt.Errorf("unable to parse address %q: %s", v, err) | |
| } | |
| addrs[i] = addr | |
| } | |
| defer func() { // convert panic in ".With" to error for better output | |
| if r := recover(); r != nil { | |
| var ok bool | |
| err, ok = r.(error) | |
| if !ok { | |
| err = fmt.Errorf("unexpected panic: %v", r) | |
| } | |
| } | |
| }() | |
| cfg := types.AccessTypeAnyOfAddresses.With(addrs...) | |
| return cfg, cfg.ValidateBasic() | |
| } | |
| } |
🤖 Prompt for AI Agents
In go/cli/wasm_parse.go around lines 245-269, the defer recovery assumes the
recovered value is an error and does a direct type assertion which can panic
again; update the deferred function to check the recovered value's type before
assignment - if r is nil do nothing, if r is an error set err = r, otherwise set
err = fmt.Errorf("%v", r) (or wrap with context) so non-error panics are
converted into an error instead of causing a secondary panic.
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (4)
proto/node/akash/wasm/v1/service.proto (1)
10-17: Fix copy‑paste module name in Msg service commentThe service comment still refers to the “market” module; this should describe the wasm Msg service instead.
-// Msg defines the market Msg service +// Msg defines the wasm Msg serviceproto/node/akash/wasm/v1/query.proto (1)
24-30: Fix copy‑paste comments for wasm Query serviceThe Query service comments still reference the “take package” and “minting parameters”, but this file is for wasm params. Suggest updating to accurately describe the module:
-// Query defines the gRPC querier service of the take package. +// Query defines the gRPC querier service of the wasm module. service Query { - // Params returns the total set of minting parameters. + // Params returns the wasm module parameters. rpc Params(QueryParamsRequest) returns (QueryParamsResponse) { option (google.api.http).get = "/akash/wasm/v1/params"; }go/cli/wasm_tx.go (1)
125-189: WireTxPersistentPreRunEforinstantiate2to avoid client context panic
GetTxWasmInstantiateContract2Cmdis the only tx command here that does not setPersistentPreRunE: TxPersistentPreRunE, yet it callsMustClientFromContextinRunE. Without the pre‑run hook, the client context won’t be initialized andakash tx wasm instantiate2will panic instead of broadcasting.func GetTxWasmInstantiateContract2Cmd() *cobra.Command { decoder := newArgDecoder(hex.DecodeString) cmd := &cobra.Command{ @@ - Aliases: []string{"start", "init", "inst", "i"}, - Args: cobra.ExactArgs(3), + Aliases: []string{"start", "init", "inst", "i"}, + Args: cobra.ExactArgs(3), + PersistentPreRunE: TxPersistentPreRunE, RunE: func(cmd *cobra.Command, args []string) error {go/cli/wasm_parse.go (1)
245-267: Make panic recovery inParseWasmAccessConfigrobust to non-error panics.The deferred recovery assumes
ris anerror(err = r.(error)), which will itself panic ifWithever panics with a non-error value, causing a confusing double-panic instead of returning a clean error.Consider:
func ParseWasmAccessConfig(raw string) (c types.AccessConfig, err error) { @@ - defer func() { // convert panic in ".With" to error for better output - if r := recover(); r != nil { - err = r.(error) - } - }() + defer func() { // convert panic in ".With" to error for better output + if r := recover(); r != nil { + if e, ok := r.(error); ok { + err = e + } else { + err = fmt.Errorf("unexpected panic in AccessTypeAnyOfAddresses.With: %v", r) + } + } + }() cfg := types.AccessTypeAnyOfAddresses.With(addrs...) return cfg, cfg.ValidateBasic()
🧹 Nitpick comments (8)
go/node/oracle/v1/params.go (1)
19-23: Consider documenting the default price feed ID.The hardcoded default AktPriceFeedId appears to be a Pyth price feed identifier. Consider adding a comment explaining what this value represents (e.g., "Default Pyth price feed ID for AKT/USD") to improve maintainability.
go/node/oracle/v1/msgs.go (4)
14-20: Consider simplifying dynamic msg type resolutionUsing
reflect.TypeOf(&MsgUpdateParams{}).Elem().Name()ininit()works, but it adds indirection for what is effectively a fixed string ("MsgUpdateParams"). A simpleconst msgTypeUpdateParams = "MsgUpdateParams"(or whatever name you want the type to be) would be clearer and avoid reflection at init-time.
33-44: ValidateBasic is good; consider using typed SDK errors for authority
ValidateBasiccorrectly checks the authority address and delegates tom.Params.Validate(), which is what we want for a params Msg. To be closer to SDK conventions, you might want to wrap the address error with a typed error (e.g., anErrInvalidAddressfrom the SDK’s errors package) instead of a genericWrap, so downstream code can inspect the error code. Example:- if _, err := sdk.AccAddressFromBech32(m.Authority); err != nil { - return cerrors.Wrap(err, "invalid authority address") - } + if _, err := sdk.AccAddressFromBech32(m.Authority); err != nil { + return cerrors.Wrap(err, "invalid authority address") // or wrap with a typed SDK error if available + }If you already have a module-specific error for invalid authority, using it here would be even better.
46-55: Confirm whether GetSignBytes is still required with your Cosmos SDK version
GetSignBytesusesModuleCdc+ JSON and is explicitly marked as deprecated in the comment. Depending on your Cosmos SDK version and how the tx signing pipeline is wired (proto-based messages vs. legacy JSON sign bytes), this method may no longer be used or needed.Given the TODO, it’s worth confirming whether:
- The current SDK version still calls
GetSignBytesanywhere for this Msg, and- You want to keep it for backward compatibility or remove it entirely.
Until then, the implementation itself is fine and matches standard patterns.
57-65: Route() implementation is OK but likely removable in newer SDKs
Route()returningRouterKeymatches oldersdk.Msginterface expectations, but your comment correctly notes thatsdk.Msgmay no longer defineRoutein newer Cosmos SDK versions. If your app has fully migrated toMsgservice routing,Route()might be dead code and could be removed once you confirm there are no legacy consumers.Given the deprecation note, I’d suggest verifying usage and then either:
- Removing
Route()(and tests) once you’re sure it’s unused, or- Documenting why it must stay (e.g., legacy handlers, migrations).
proto/node/akash/wasm/v1/genesis.proto (1)
9-16: Clarify GenesisState comment to match the actual schema
GenesisStatecontains a singleParamsfield, not a “slice” of parameters. Consider tightening the wording to avoid confusion:-// GenesisState stores slice of genesis wasm parameters. +// GenesisState stores the genesis parameters for the wasm module. message GenesisState { - // Params holds parameters of the genesis of akash wasm. + // Params holds the wasm module parameters at genesis. Params params = 1 [go/cli/wasm_parse.go (1)
271-296: Tighten parsing of access-config updates and improve diagnostics.Right now
strings.Split(c, ":")and the generic"invalid format"error make it hard to see which argument was malformed and don’t handle additional:characters gracefully.You can make this a bit more robust and debuggable:
- parts := strings.Split(c, ":") - if len(parts) != 2 { - return nil, fmt.Errorf("invalid format") - } + parts := strings.SplitN(c, ":", 2) + if len(parts) != 2 { + return nil, fmt.Errorf("invalid format %q, expected code_id:access_config", c) + }go/cli/testdata/wasm/contracts.go (1)
11-52: Consider returning copies of embedded wasm blobs to avoid accidental mutation.The helpers currently return the backing
[]byteslices from the embedded globals, so tests that modify these slices will mutate the shared data for all callers.If you want stronger safety (especially in larger test suites), you can defensively copy:
func ReflectContractWasm() []byte { - return reflectContract + return append([]byte(nil), reflectContract...) } @@ func HackatomContractWasm() []byte { - return hackatomContract + return append([]byte(nil), hackatomContract...) }(Same pattern can be applied to the other getters if desired.)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (25)
go/cli/testdata/wasm/burner.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/cyberpunk.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/hackatom.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/hackatom_42.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/hackatom_420.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/ibc_reflect.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/queue.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/reflect_1_1.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/reflect_1_5.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/reflect_2_0.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/replier.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/staking.wasmis excluded by!**/*.wasmgo/node/oracle/v1/genesis.pb.gois excluded by!**/*.pb.gogo/node/oracle/v1/params.pb.gois excluded by!**/*.pb.gogo/node/oracle/v1/paramsmsg.pb.gois excluded by!**/*.pb.gogo/node/oracle/v1/query.pb.gois excluded by!**/*.pb.gogo/node/oracle/v1/query.pb.gw.gois excluded by!**/*.pb.gw.gogo/node/oracle/v1/service.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/event.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/genesis.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/params.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/paramsmsg.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/query.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/query.pb.gw.gois excluded by!**/*.pb.gw.gogo/node/wasm/v1/service.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (26)
go/cli/module_query.go(1 hunks)go/cli/testdata/wasm/contracts.go(1 hunks)go/cli/testdata/wasm/download_releases.sh(1 hunks)go/cli/wasm_parse.go(1 hunks)go/cli/wasm_query.go(1 hunks)go/cli/wasm_tx.go(1 hunks)go/cli/wasm_tx_test.go(1 hunks)go/node/oracle/v1/codec.go(1 hunks)go/node/oracle/v1/key.go(1 hunks)go/node/oracle/v1/msgs.go(1 hunks)go/node/oracle/v1/params.go(1 hunks)go/node/wasm/v1/codec.go(1 hunks)go/node/wasm/v1/key.go(1 hunks)go/node/wasm/v1/msgs.go(1 hunks)go/node/wasm/v1/params.go(1 hunks)proto/node/akash/oracle/v1/genesis.proto(1 hunks)proto/node/akash/oracle/v1/params.proto(1 hunks)proto/node/akash/oracle/v1/paramsmsg.proto(1 hunks)proto/node/akash/oracle/v1/query.proto(1 hunks)proto/node/akash/oracle/v1/service.proto(1 hunks)proto/node/akash/wasm/v1/event.proto(1 hunks)proto/node/akash/wasm/v1/genesis.proto(1 hunks)proto/node/akash/wasm/v1/params.proto(1 hunks)proto/node/akash/wasm/v1/paramsmsg.proto(1 hunks)proto/node/akash/wasm/v1/query.proto(1 hunks)proto/node/akash/wasm/v1/service.proto(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- go/node/wasm/v1/key.go
- go/cli/module_query.go
- go/node/wasm/v1/msgs.go
- go/cli/wasm_query.go
- go/cli/testdata/wasm/download_releases.sh
- go/node/wasm/v1/params.go
- go/node/wasm/v1/codec.go
🧰 Additional context used
🧬 Code graph analysis (6)
go/node/oracle/v1/params.go (2)
go/node/oracle/v1/params.pb.go (3)
Params(28-39)Params(42-42)Params(43-45)go/node/wasm/v1/params.go (2)
ParamKeyTable(11-13)DefaultParams(19-21)
go/cli/wasm_tx_test.go (1)
go/cli/wasm_parse.go (4)
ParseWasmStoreCodeArgs(74-101)ParseWasmVerificationFlags(24-71)ParseWasmAccessConfigFlags(103-155)ParseStoreCodeGrants(298-325)
go/node/oracle/v1/key.go (1)
go/node/wasm/v1/key.go (2)
ModuleName(5-5)ParamsPrefix(14-16)
go/node/oracle/v1/msgs.go (4)
go/node/oracle/v1/paramsmsg.pb.go (3)
MsgUpdateParams(31-38)MsgUpdateParams(42-42)MsgUpdateParams(43-45)go/node/oracle/v1/params.pb.go (3)
Params(28-39)Params(42-42)Params(43-45)go/node/oracle/v1/codec.go (1)
ModuleCdc(19-19)go/node/oracle/v1/key.go (1)
RouterKey(11-11)
go/node/oracle/v1/codec.go (3)
go/node/wasm/v1/codec.go (2)
RegisterLegacyAminoCodec(25-27)RegisterInterfaces(30-36)go/node/oracle/v1/paramsmsg.pb.go (3)
MsgUpdateParams(31-38)MsgUpdateParams(42-42)MsgUpdateParams(43-45)go/node/wasm/v1/key.go (1)
ModuleName(5-5)
go/cli/wasm_parse.go (1)
go/cli/flags/flags.go (1)
FlagAmount(163-163)
🪛 Buf (1.60.0)
proto/node/akash/oracle/v1/service.proto
4-4: import "cosmos/msg/v1/msg.proto": file does not exist
(COMPILE)
proto/node/akash/wasm/v1/params.proto
5-5: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/oracle/v1/params.proto
5-5: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/oracle/v1/query.proto
5-5: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/wasm/v1/genesis.proto
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/wasm/v1/paramsmsg.proto
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/oracle/v1/genesis.proto
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/wasm/v1/query.proto
5-5: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/oracle/v1/paramsmsg.proto
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/wasm/v1/event.proto
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/wasm/v1/service.proto
4-4: import "cosmos/msg/v1/msg.proto": file does not exist
(COMPILE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: test
- GitHub Check: coverage
- GitHub Check: go
- GitHub Check: proto
- GitHub Check: breakage
🔇 Additional comments (12)
go/node/oracle/v1/key.go (1)
1-16: LGTM!The module constants and ParamsPrefix function follow standard Cosmos SDK conventions and are consistent with the wasm module pattern.
proto/node/akash/oracle/v1/paramsmsg.proto (1)
1-36: LGTM!The MsgUpdateParams message follows standard Cosmos SDK governance patterns with proper authority signer annotation and non-nullable params field.
go/node/oracle/v1/msgs.go (2)
10-12: Compile-timesdk.Msgassertion looks goodThe interface assertion against
sdk.Msgis idiomatic and helps catch signature drift at compile time. No changes needed here.
24-25: Type() implementation is fine given current pattern
Type()correctly returns the module-levelmsgTypeUpdateParamsand matches the usual Cosmos SDK Msg pattern. This is consistent with the chosen approach in this file.proto/node/akash/wasm/v1/service.proto (1)
4-8: Verify Buf config forcosmos/msg/v1/msg.protoimportBuf reports
import "cosmos/msg/v1/msg.proto": file does not exist. The path itself matches standard Cosmos SDK protos, so this likely means the buf module/deps aren’t wired to include the Cosmos SDK protos (or the vendored path differs). Please verify your buf configuration or vendored proto layout so this import (and similar ones) resolve correctly.proto/node/akash/wasm/v1/paramsmsg.proto (1)
16-30: MsgUpdateParams proto wiring looks consistentSigner option, authority field, and non‑nullable
Paramsare wired as expected for a gov‑driven params update; no issues from a proto/API perspective.proto/node/akash/wasm/v1/params.proto (1)
5-16: Params definition looks good; verify gogoproto import resolutionThe
Paramsmessage andblocked_addressesfield (includingcosmos.AddressStringscalar and JSON/YAML tags) look correct and consistent with other modules.Buf’s “import
gogoproto/gogo.proto: file does not exist” warning likely means the gogoproto definitions aren’t in this buf module’s include path or deps, rather than an issue with this file. Please confirm that gogoproto is vendored or declared as a buf dependency so this import resolves.proto/node/akash/wasm/v1/event.proto (1)
9-28: EventMsgBlocked event schema looks solidField selection and annotations (
contract_addressasAddressString,msg_type,reason, plus gogoproto tags andequal = true) are consistent and ready for indexing/logging use; no changes needed.go/cli/wasm_tx_test.go (1)
18-233: Good coverage of wasm CLI parsing helpersThese tests exercise the key happy‑paths and error conditions for verification flags, access configs, and store‑code grants (including gzip vs raw wasm, checksum mismatch, address parsing, and wildcard patterns). The expectations line up with the current helper implementations.
go/cli/wasm_parse.go (2)
24-243: Store/instantiate/execute argument parsing looks solid.The flag extraction, validation (including label/admin/no-admin invariants), wasm/gzip handling, and message construction for store/instantiate/execute all look consistent and correct; I don’t see functional or safety issues in these paths.
298-323: No changes needed; the current implementation is correct.The
CodeHashfield inParseStoreCodeGrantsis intentionally designed to store opaque string identifiers as raw bytes, not hex-encoded values. This is confirmed by comprehensive unit tests inwasm_tx_test.go(lines 132–204), which explicitly verify thatCodeHashshould be[]byte("*")for wildcards and[]byte("any_checksum_1")for arbitrary identifiers—not hex-decoded bytes.The apparent "asymmetry" with verification flags (line 33, using
flags.GetBytesHex) reflects different use cases: verification flags decode actual SHA256 hashes from WASM files, whileParseStoreCodeGrantsparses user-provided grant rules where the code hash is a flexible identifier (including wildcard*). Applyinghex.DecodeStringas suggested would break all existing tests and misrepresent the intended functionality.Likely an incorrect or invalid review comment.
go/cli/testdata/wasm/contracts.go (1)
54-99: Embedded message/response structs look appropriate for test usage.The reflect/owner/query payload and response types are simple, JSON-friendly wrappers around the wasmvm types and addresses; they’re suitable for constructing test messages, and I don’t see correctness issues here.
| // ModuleCdc references the global x/deployment module codec. Note, the codec should | ||
| // ONLY be used in certain instances of tests and for JSON encoding as Amino is | ||
| // still used for that purpose. | ||
| // | ||
| // The actual codec used for serialization should be provided to x/deployment and | ||
| // defined at the application level. |
There was a problem hiding this comment.
Fix module references in comments.
The comments reference "x/deployment module" but this is the oracle module.
Apply this diff:
var (
- // ModuleCdc references the global x/deployment module codec. Note, the codec should
+ // ModuleCdc references the global x/oracle module codec. Note, the codec should
// ONLY be used in certain instances of tests and for JSON encoding as Amino is
// still used for that purpose.
//
- // The actual codec used for serialization should be provided to x/deployment and
+ // The actual codec used for serialization should be provided to x/oracle and
// defined at the application level.📝 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.
| // ModuleCdc references the global x/deployment module codec. Note, the codec should | |
| // ONLY be used in certain instances of tests and for JSON encoding as Amino is | |
| // still used for that purpose. | |
| // | |
| // The actual codec used for serialization should be provided to x/deployment and | |
| // defined at the application level. | |
| // ModuleCdc references the global x/oracle module codec. Note, the codec should | |
| // ONLY be used in certain instances of tests and for JSON encoding as Amino is | |
| // still used for that purpose. | |
| // | |
| // The actual codec used for serialization should be provided to x/oracle and | |
| // defined at the application level. |
🤖 Prompt for AI Agents
In go/node/oracle/v1/codec.go around lines 11 to 16, update the comment
references from "x/deployment module" to "x/oracle module" (or simply "oracle
module") so the doc correctly names the module; keep the rest of the comment
unchanged and ensure casing/terminology matches other files (e.g., "oracle
module codec", "x/oracle") for consistency.
| // GetSigners returns the expected signers for a MsgUpdateParams message. | ||
| func (m *MsgUpdateParams) GetSigners() []sdk.AccAddress { | ||
| addr, _ := sdk.AccAddressFromBech32(m.Authority) | ||
| return []sdk.AccAddress{addr} | ||
| } |
There was a problem hiding this comment.
Don’t ignore address parse errors in GetSigners
GetSigners currently ignores the error from sdk.AccAddressFromBech32 and will silently return a zero-value address if Authority is invalid. This can lead to incorrect signer sets and is different from the usual Cosmos pattern of panicking on invalid addresses (since messages should never reach this point with bad signers).
I recommend handling the error explicitly and panicking if it occurs, which is what most SDK modules do:
func (m *MsgUpdateParams) GetSigners() []sdk.AccAddress {
- addr, _ := sdk.AccAddressFromBech32(m.Authority)
- return []sdk.AccAddress{addr}
+ addr, err := sdk.AccAddressFromBech32(m.Authority)
+ if err != nil {
+ panic(err)
+ }
+ return []sdk.AccAddress{addr}
}🤖 Prompt for AI Agents
In go/node/oracle/v1/msgs.go around lines 27 to 31, GetSigners currently ignores
the error returned by sdk.AccAddressFromBech32; change it to check the error and
panic on failure (matching Cosmos SDK conventions) — e.g. call
sdk.AccAddressFromBech32(m.Authority), if err != nil panic(fmt.Sprintf("invalid
authority address: %s, %v", m.Authority, err)), otherwise return the address
slice; add the fmt import if not already present.
|
|
||
| var _ paramtypes.ParamSet = (*Params)(nil) | ||
|
|
||
| // ParamKeyTable for take module |
There was a problem hiding this comment.
Fix the module name in the comment.
The comment references "take module" but this is the oracle module.
Apply this diff:
-// ParamKeyTable for take module
+// ParamKeyTable for oracle module📝 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.
| // ParamKeyTable for take module | |
| // ParamKeyTable for oracle module |
🤖 Prompt for AI Agents
In go/node/oracle/v1/params.go around line 9, the comment above ParamKeyTable
incorrectly says "take module"; update the comment to reference the correct
module name ("oracle module") by replacing "take module" with "oracle module" so
the comment accurately describes ParamKeyTable for the oracle module.
| func (p Params) Validate() error { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Implement parameter validation.
The Validate() method currently performs no validation. Based on the Params structure (which includes fields like akt_price_feed_id, pyth_contract_address, price_update_frequency, max_price_age, and enabled), consider adding validation for:
- Non-empty required string fields (price_feed_id, contract address)
- Valid format for price feed IDs and addresses
- Reasonable bounds for frequency and age parameters
Invalid parameters could cause runtime issues or unexpected behavior.
Would you like me to generate a validation implementation based on the Params structure?
🤖 Prompt for AI Agents
In go/node/oracle/v1/params.go around lines 25 to 27, the Params.Validate() stub
returns nil and must enforce schema rules: check required string fields
(price_feed_id and pyth_contract_address) are non-empty, validate their formats
(e.g., price_feed_id matches expected pattern/length and pyth_contract_address
is a valid hex/bech32 address as used in the project), ensure numeric fields
price_update_frequency and max_price_age are within reasonable bounds (>0 and
below configured maxs), and validate enabled is a boolean (implicitly OK) — on
any failure return a descriptive error (include field name and reason).
Implement these checks and unit-test edge cases (empty, malformed IDs/addresses,
zero/overflow frequencies and ages).
|
|
||
| // GenesisState stores slice of genesis parameters. | ||
| message GenesisState { | ||
| // Params holds parameters of the genesis of akash params module. |
There was a problem hiding this comment.
Fix the module name in the comment.
The comment references "akash params module" but this is the oracle module.
Apply this diff:
- // Params holds parameters of the genesis of akash params module.
+ // Params holds parameters of the genesis of the oracle module.📝 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.
| // Params holds parameters of the genesis of akash params module. | |
| // Params holds parameters of the genesis of the oracle module. |
🤖 Prompt for AI Agents
In proto/node/akash/oracle/v1/genesis.proto around line 11, the comment
incorrectly refers to the "akash params module"; update the comment to reference
the "oracle params module" instead. Edit the line to replace "akash params
module" with "oracle module" (or "oracle params module" if you want to keep
"params" wording) so the comment accurately describes the genesis parameters for
the oracle module.
| message Params { | ||
| option (gogoproto.equal) = true; | ||
| option (gogoproto.goproto_stringer) = false; | ||
|
|
||
| // akt_price_feed_id is the Pyth price feed identifier for AKT/USD | ||
| string akt_price_feed_id = 1; | ||
| } |
There was a problem hiding this comment.
Add missing fields to Params message.
Based on the generated code context (params.pb.go), the Params message is incomplete. It should include:
- pyth_contract_address (string)
- price_update_frequency (uint64)
- max_price_age (uint64)
- enabled (bool)
The proto definition currently only has akt_price_feed_id. This mismatch will cause code generation and runtime issues.
Apply this diff:
// Params defines the parameters for the x/params package.
message Params {
option (gogoproto.equal) = true;
option (gogoproto.goproto_stringer) = false;
+ // pyth_contract_address is the address of Pyth contract on Akash
+ string pyth_contract_address = 1;
+
// akt_price_feed_id is the Pyth price feed identifier for AKT/USD
- string akt_price_feed_id = 1;
+ string akt_price_feed_id = 2;
+
+ // price_update_frequency is how often prices should be updated (in seconds)
+ uint64 price_update_frequency = 3;
+
+ // max_price_age is the maximum age of price data to accept (in seconds)
+ uint64 max_price_age = 4;
+
+ // enabled determines if the oracle module is active
+ bool enabled = 5;
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In proto/node/akash/oracle/v1/params.proto around lines 11 to 17, the Params
message only defines akt_price_feed_id and is missing additional fields expected
by generated code; update the message to add pyth_contract_address (string) as
field 2, price_update_frequency (uint64) as field 3, max_price_age (uint64) as
field 4, and enabled (bool) as field 5 so the proto matches the generated
params.pb.go and avoids codegen/runtime mismatches.
| // QueryPriceFeedConfigRequest is the request type for price feed config. | ||
| message QueryPriceFeedConfigRequest { | ||
| string denom = 1; | ||
| } | ||
|
|
||
| // QueryPriceFeedConfigResponse is the response type for price feed config. | ||
| message QueryPriceFeedConfigResponse { | ||
| string price_feed_id = 1; | ||
| string pyth_contract_address = 2; | ||
| bool enabled = 3; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Move query message definitions to query.proto.
QueryPriceFeedConfigRequest and QueryPriceFeedConfigResponse should be defined in proto/node/akash/oracle/v1/query.proto rather than params.proto. This follows the standard Cosmos SDK proto organization pattern where query.proto contains all query-related messages and params.proto contains only parameter definitions.
🤖 Prompt for AI Agents
In proto/node/akash/oracle/v1/params.proto around lines 19-29, the two query
message types QueryPriceFeedConfigRequest and QueryPriceFeedConfigResponse
belong in the module's query.proto file; remove these message definitions from
params.proto, add identical message definitions into
proto/node/akash/oracle/v1/query.proto (keeping the same field numbers and
types), update any imports/option lines if needed, and ensure any references
(RPC service methods, other protos, and generated code) point to the moved
messages; after the move, regenerate the protobuf/TS/Go artifacts to reflect the
change.
| // Query defines the gRPC querier service of the take package. | ||
| service Query { | ||
| // Params returns the total set of minting parameters. |
There was a problem hiding this comment.
Fix incorrect comments.
The comments contain references to the wrong module/functionality.
Apply this diff:
-// Query defines the gRPC querier service of the take package.
+// Query defines the gRPC querier service of the oracle module.
service Query {
- // Params returns the total set of minting parameters.
+ // Params returns the oracle module parameters.
rpc Params(QueryParamsRequest) returns (QueryParamsResponse) {📝 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.
| // Query defines the gRPC querier service of the take package. | |
| service Query { | |
| // Params returns the total set of minting parameters. | |
| // Query defines the gRPC querier service of the oracle module. | |
| service Query { | |
| // Params returns the oracle module parameters. |
🤖 Prompt for AI Agents
In proto/node/akash/oracle/v1/query.proto around lines 24 to 26, the service
comments incorrectly reference the "take" package and "minting parameters";
update the comment to accurately describe the oracle module: change the service
description to "Query defines the gRPC querier service of the oracle package."
and update the Params line to a correct description such as "Params returns the
total set of oracle parameters." ensuring wording matches repository comment
style.
| // UpdateParams defines a governance operation for updating the x/wasm module | ||
| // parameters. The authority is hard-coded to the x/gov module account. |
There was a problem hiding this comment.
Fix the module name in the comment.
The comment references "x/wasm module" but this should be "x/oracle module".
Apply this diff:
- // UpdateParams defines a governance operation for updating the x/wasm module
+ // UpdateParams defines a governance operation for updating the x/oracle module📝 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.
| // UpdateParams defines a governance operation for updating the x/wasm module | |
| // parameters. The authority is hard-coded to the x/gov module account. | |
| // UpdateParams defines a governance operation for updating the x/oracle module | |
| // parameters. The authority is hard-coded to the x/gov module account. |
🤖 Prompt for AI Agents
In proto/node/akash/oracle/v1/service.proto around lines 13 to 14, the comment
incorrectly references "x/wasm module" — update the text to "x/oracle module" so
the comment correctly reads that UpdateParams defines a governance operation for
updating the x/oracle module parameters; keep the rest of the comment intact and
ensure no other mentions of "wasm" in that sentence remain.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (16)
go/cli/wasm_tx.go (1)
126-189: MissingPersistentPreRunEwill cause panic.The
instantiate2command is missing thePersistentPreRunE: TxPersistentPreRunEhook that all other tx commands have. This will causeMustClientFromContexton line 145 to panic because the client context won't be initialized.Apply this diff to add the missing hook:
func GetTxWasmInstantiateContract2Cmd() *cobra.Command { decoder := newArgDecoder(hex.DecodeString) cmd := &cobra.Command{ Use: "instantiate2 [code_id_int64] [json_encoded_init_args] [salt] --label [text] --admin [address,optional] --amount [coins,optional] " + "--fix-msg [bool,optional]", Short: "Instantiate a wasm contract with predictable address", Long: fmt.Sprintf(`Creates a new instance of an uploaded wasm code with the given 'constructor' message. Each contract instance has a unique address assigned. They are assigned automatically but in order to have predictable addresses for special use cases, the given 'salt' argument and '--fix-msg' parameters can be used to generate a custom address. Predictable address example (also see '%s query wasm build-address -h'): $ %s tx wasm instantiate2 1 '{"foo":"bar"}' $(echo -n "testing" | xxd -ps) --admin="$(%s keys show mykey -a)" \ --from mykey --amount="100ustake" --label "local0.1.0" \ --fix-msg `, version.AppName, version.AppName, version.AppName), - Aliases: []string{"start", "init", "inst", "i"}, - Args: cobra.ExactArgs(3), + Aliases: []string{"start", "init", "inst", "i"}, + Args: cobra.ExactArgs(3), + PersistentPreRunE: TxPersistentPreRunE, RunE: func(cmd *cobra.Command, args []string) error {go/cli/wasm_parse.go (1)
261-265: Unsafe type assertion in panic recovery.The defer recovery assumes the recovered panic value is an
errortype. If.With()panics with a non-error value (e.g., a string), this will cause a secondary panic.Apply this diff to add a type check:
defer func() { // convert panic in ".With" to error for better output if r := recover(); r != nil { - err = r.(error) + var ok bool + if err, ok = r.(error); !ok { + err = fmt.Errorf("unexpected panic: %v", r) + } } }()proto/node/akash/wasm/v1/service.proto (1)
10-10: Fix copy-paste error in comment.The comment references "market Msg service" but this is the wasm module service.
Apply this diff:
-// Msg defines the market Msg service +// Msg defines the wasm Msg servicego/node/oracle/v1/params.go (2)
9-10: Fix the module name in the comment.The comment references "take module" but this is the oracle module.
Apply this diff:
-// ParamKeyTable for take module +// ParamKeyTable for oracle module
25-27: Implement parameter validation.The Validate() method currently performs no validation. Based on the Params structure (which includes fields like pyth_contract_address, akt_price_feed_id, price_update_frequency, max_price_age, and enabled), consider adding validation for:
- Non-empty required string fields (pyth_contract_address, akt_price_feed_id)
- Valid format for addresses and price feed IDs
- Reasonable bounds for frequency and age parameters (e.g., > 0)
Invalid parameters could cause runtime issues or unexpected behavior in the oracle module.
Would you like me to generate a validation implementation based on the Params structure?
proto/node/akash/oracle/v1/params.proto (2)
19-29: Consider movingQueryPriceFeedConfig*messages intoquery.proto.These are purely query request/response types and are referenced from
oracle/v1/query.proto; per common Cosmos proto layout they typically live alongside theQueryservice rather than inparams.proto.This is non-blocking, but moving them would improve discoverability and match the usual module proto organization.
10-17: EnsureParamsincludes all intended oracle configuration fields.Right now
Paramsonly exposesakt_price_feed_id. Earlier generatedparams.pb.goand prior review suggested additional fields (e.g., Pyth contract address, update frequency, max price age, enabled flag); if those are still required by the Go types or module logic, they need to be declared here with stable field numbers to avoid codegen/runtime mismatches.If the current design has changed and only
akt_price_feed_idis needed, please confirm that the generated GoParamsstruct and any genesis/query handling have been regenerated from this proto and no longer expect the extra fields.proto/node/akash/oracle/v1/service.proto (1)
13-14: Fix module name inUpdateParamscomment.This RPC updates oracle params, not wasm params; the comment should reference the x/oracle module.
- // UpdateParams defines a governance operation for updating the x/wasm module + // UpdateParams defines a governance operation for updating the x/oracle module // parameters. The authority is hard-coded to the x/gov module account.go/node/wasm/v1/params.go (2)
9-10: Fix copy‑paste in ParamKeyTable comment (take → wasm).These comments should refer to the wasm module/store, not the take module/store.
-// ParamKeyTable for take module -// Deprecated: now params can be accessed on key `0x01` on the take store. +// ParamKeyTable for wasm module +// Deprecated: now params can be accessed on key `0x01` on the wasm store.
15-25: EmptyParamSetPairsandValidatemake wasm params effectively unvalidated.
ParamSetPairsreturns an empty set andValidatereturns nil. IfParamscurrently has real configuration fields (e.g. the previously‑notedBlockedAddresses), they’re neither registered with the params key table nor validated, yetMsgUpdateParams.ValidateBasicstill callsParams.Validate(). That means governance proposals can push invalid config that only fails much later at runtime.Please either:
- Register any actual Params fields in
ParamSetPairswith appropriate validators and implementValidate(), or- If wasm no longer uses x/params at all, remove the
ParamSetwiring (and the call fromMsgUpdateParams.ValidateBasic) to avoid the misleading no‑op.proto/node/akash/oracle/v1/genesis.proto (1)
11-11: Fix module name in GenesisStateParamscomment.This field holds parameters for the oracle module, not a generic “akash params module”.
- // Params holds parameters of the genesis of akash params module. + // Params holds parameters of the genesis of the oracle module.go/node/oracle/v1/codec.go (1)
11-16: UpdateModuleCdccomment to reference x/oracle, not x/deployment.The codec here belongs to the oracle module; the current wording still references deployment.
- // ModuleCdc references the global x/deployment module codec. Note, the codec should + // ModuleCdc references the global x/oracle module codec. Note, the codec should @@ - // The actual codec used for serialization should be provided to x/deployment and + // The actual codec used for serialization should be provided to x/oracle andproto/node/akash/oracle/v1/query.proto (1)
24-27: Fix oracleQueryservice comments (take/minting → oracle/params).The service and RPC are for the oracle module, not the take package or minting parameters.
-// Query defines the gRPC querier service of the take package. +// Query defines the gRPC querier service of the oracle module. service Query { - // Params returns the total set of minting parameters. - rpc Params(QueryParamsRequest) returns (QueryParamsResponse) { + // Params returns the oracle module parameters. + rpc Params(QueryParamsRequest) returns (QueryParamsResponse) {proto/node/akash/wasm/v1/query.proto (1)
24-27: Fix wasmQueryservice comments (take/minting → wasm/params).These comments were copy‑pasted from another module; they should describe the wasm querier and its params.
-// Query defines the gRPC querier service of the take package. +// Query defines the gRPC querier service of the wasm module. service Query { - // Params returns the total set of minting parameters. - rpc Params(QueryParamsRequest) returns (QueryParamsResponse) { + // Params returns the wasm module parameters. + rpc Params(QueryParamsRequest) returns (QueryParamsResponse) {go/node/wasm/v1/msgs.go (1)
27-31: Handle Bech32 decode failures inGetSignersinstead of returning a nil signer.
GetSignerscurrently discards the error fromsdk.AccAddressFromBech32and may return a nil address whenAuthorityis malformed; since BaseApp collects signers beforeValidateBasic, this can surface as confusing runtime failures. Decode should be strict here.func (m *MsgUpdateParams) GetSigners() []sdk.AccAddress { - addr, _ := sdk.AccAddressFromBech32(m.Authority) - return []sdk.AccAddress{addr} + addr, err := sdk.AccAddressFromBech32(m.Authority) + if err != nil { + // Consistent with Cosmos SDK patterns: invalid addresses here are programmer/config errors. + panic(err) + } + return []sdk.AccAddress{addr} }Please verify this matches the conventions for your Cosmos SDK version (panic vs. returning an error in a different hook) and adjust accordingly if your codebase uses a different pattern.
go/node/oracle/v1/msgs.go (1)
27-31: Don't ignore address parse errors in GetSigners.
GetSignerscurrently ignores the error fromsdk.AccAddressFromBech32and will silently return a zero-value address ifAuthorityis invalid. This can lead to incorrect signer sets and violates the standard Cosmos SDK pattern of panicking on invalid addresses (since messages should never reach this point with bad signers).Apply this diff to handle the error explicitly:
func (m *MsgUpdateParams) GetSigners() []sdk.AccAddress { - addr, _ := sdk.AccAddressFromBech32(m.Authority) - return []sdk.AccAddress{addr} + addr, err := sdk.AccAddressFromBech32(m.Authority) + if err != nil { + panic(err) + } + return []sdk.AccAddress{addr} }
🧹 Nitpick comments (5)
go/cli/wasm_tx.go (1)
256-272: Consider validating contract address format.The
parseMigrateContractArgsfunction doesn't validate thatargs[0]is a valid bech32 contract address before constructing the message. WhileValidateBasic()may catch this, early validation with a descriptive error would improve UX.func parseMigrateContractArgs(args []string, sender string) (types.MsgMigrateContract, error) { + // Validate contract address + if _, err := sdk.AccAddressFromBech32(args[0]); err != nil { + return types.MsgMigrateContract{}, errorsmod.Wrap(err, "contract address") + } + // get the id of the code to instantiate codeID, err := strconv.ParseUint(args[1], 10, 64)go/cli/wasm_parse.go (1)
226-243: MissingValidateBasic()call for consistency.Unlike
ParseWasmStoreCodeArgsandParseWasmInstantiateArgs, this function doesn't callValidateBasic()on the constructed message. This inconsistency could lead to invalid messages being returned.func ParseWasmExecuteArgs(contractAddr, execMsg string, sender sdk.AccAddress, flags *flag.FlagSet) (types.MsgExecuteContract, error) { amountStr, err := flags.GetString(cflags.FlagAmount) if err != nil { return types.MsgExecuteContract{}, fmt.Errorf("amount: %s", err) } amount, err := sdk.ParseCoinsNormalized(amountStr) if err != nil { return types.MsgExecuteContract{}, err } - return types.MsgExecuteContract{ + msg := types.MsgExecuteContract{ Sender: sender.String(), Contract: contractAddr, Funds: amount, Msg: []byte(execMsg), - }, nil + } + return msg, msg.ValidateBasic() }proto/node/akash/oracle/v1/service.proto (1)
10-11: Clarify service description to reference oracle, not market.The
Msgservice here belongs to the oracle module, so the doc should say that explicitly.-// Msg defines the market Msg service +// Msg defines the oracle Msg serviceproto/node/akash/oracle/v1/params.proto (1)
10-10: AdjustParamsdocstring to reference the oracle module.The comment still mentions the generic x/params package; here it’s defining oracle module parameters.
-// Params defines the parameters for the x/params package. +// Params defines the parameters for the oracle module.go/node/oracle/v1/msgs.go (1)
14-20: Replace reflection with a simple constant.Using reflection to derive the message type name adds unnecessary complexity and runtime overhead. In Cosmos SDK modules, the type name is typically defined as a simple constant since it's known at compile time.
Apply this diff to simplify the type name resolution:
-var ( - msgTypeUpdateParams = "" -) - -func init() { - msgTypeUpdateParams = reflect.TypeOf(&MsgUpdateParams{}).Elem().Name() -} +const ( + msgTypeUpdateParams = "MsgUpdateParams" +)You can also remove the
reflectimport if it's not used elsewhere:import ( - "reflect" - cerrors "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" )
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (28)
go/cli/go.sumis excluded by!**/*.sumgo/cli/testdata/wasm/burner.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/cyberpunk.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/hackatom.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/hackatom_42.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/hackatom_420.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/ibc_reflect.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/queue.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/reflect_1_1.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/reflect_1_5.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/reflect_2_0.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/replier.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/staking.wasmis excluded by!**/*.wasmgo/go.sumis excluded by!**/*.sumgo/node/oracle/v1/genesis.pb.gois excluded by!**/*.pb.gogo/node/oracle/v1/params.pb.gois excluded by!**/*.pb.gogo/node/oracle/v1/paramsmsg.pb.gois excluded by!**/*.pb.gogo/node/oracle/v1/query.pb.gois excluded by!**/*.pb.gogo/node/oracle/v1/query.pb.gw.gois excluded by!**/*.pb.gw.gogo/node/oracle/v1/service.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/event.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/genesis.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/params.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/paramsmsg.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/query.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/query.pb.gw.gois excluded by!**/*.pb.gw.gogo/node/wasm/v1/service.pb.gois excluded by!**/*.pb.gogo/sdl/go.sumis excluded by!**/*.sum
📒 Files selected for processing (29)
go/cli/go.mod(12 hunks)go/cli/module_query.go(1 hunks)go/cli/testdata/wasm/contracts.go(1 hunks)go/cli/testdata/wasm/download_releases.sh(1 hunks)go/cli/wasm_parse.go(1 hunks)go/cli/wasm_query.go(1 hunks)go/cli/wasm_tx.go(1 hunks)go/cli/wasm_tx_test.go(1 hunks)go/go.mod(1 hunks)go/node/oracle/v1/codec.go(1 hunks)go/node/oracle/v1/key.go(1 hunks)go/node/oracle/v1/msgs.go(1 hunks)go/node/oracle/v1/params.go(1 hunks)go/node/wasm/v1/codec.go(1 hunks)go/node/wasm/v1/key.go(1 hunks)go/node/wasm/v1/msgs.go(1 hunks)go/node/wasm/v1/params.go(1 hunks)go/sdl/go.mod(1 hunks)proto/node/akash/oracle/v1/genesis.proto(1 hunks)proto/node/akash/oracle/v1/params.proto(1 hunks)proto/node/akash/oracle/v1/paramsmsg.proto(1 hunks)proto/node/akash/oracle/v1/query.proto(1 hunks)proto/node/akash/oracle/v1/service.proto(1 hunks)proto/node/akash/wasm/v1/event.proto(1 hunks)proto/node/akash/wasm/v1/genesis.proto(1 hunks)proto/node/akash/wasm/v1/params.proto(1 hunks)proto/node/akash/wasm/v1/paramsmsg.proto(1 hunks)proto/node/akash/wasm/v1/query.proto(1 hunks)proto/node/akash/wasm/v1/service.proto(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- go/cli/module_query.go
- go/node/wasm/v1/key.go
- go/cli/wasm_query.go
- go/cli/wasm_tx_test.go
- go/node/oracle/v1/key.go
- go/node/wasm/v1/codec.go
- go/cli/testdata/wasm/download_releases.sh
🧰 Additional context used
🧬 Code graph analysis (5)
go/node/oracle/v1/codec.go (3)
go/node/wasm/v1/codec.go (2)
RegisterLegacyAminoCodec(25-27)RegisterInterfaces(30-36)go/node/oracle/v1/paramsmsg.pb.go (3)
MsgUpdateParams(31-38)MsgUpdateParams(42-42)MsgUpdateParams(43-45)go/node/wasm/v1/paramsmsg.pb.go (3)
MsgUpdateParams(31-38)MsgUpdateParams(42-42)MsgUpdateParams(43-45)
go/node/oracle/v1/params.go (2)
go/node/oracle/v1/params.pb.go (3)
Params(28-39)Params(42-42)Params(43-45)go/node/wasm/v1/params.go (2)
ParamKeyTable(11-13)DefaultParams(19-21)
go/cli/wasm_tx.go (4)
go/cli/tx.go (1)
TxPersistentPreRunE(13-52)go/cli/context.go (1)
MustClientFromContext(47-54)go/cli/wasm_parse.go (4)
ParseWasmStoreCodeArgs(74-101)ParseWasmInstantiateArgs(157-224)ParseWasmExecuteArgs(226-243)ParseWasmAccessConfigFlags(103-155)go/cli/flags/flags.go (2)
AddTxFlagsToCmd(315-352)FlagAmount(163-163)
go/node/oracle/v1/msgs.go (3)
go/node/oracle/v1/paramsmsg.pb.go (3)
MsgUpdateParams(31-38)MsgUpdateParams(42-42)MsgUpdateParams(43-45)go/node/oracle/v1/codec.go (1)
ModuleCdc(19-19)go/node/oracle/v1/key.go (1)
RouterKey(11-11)
go/cli/wasm_parse.go (1)
go/cli/flags/flags.go (1)
FlagAmount(163-163)
🪛 Buf (1.60.0)
proto/node/akash/oracle/v1/query.proto
5-5: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/oracle/v1/genesis.proto
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/oracle/v1/paramsmsg.proto
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/wasm/v1/event.proto
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/wasm/v1/genesis.proto
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/oracle/v1/params.proto
5-5: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/oracle/v1/service.proto
4-4: import "cosmos/msg/v1/msg.proto": file does not exist
(COMPILE)
proto/node/akash/wasm/v1/params.proto
5-5: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/wasm/v1/paramsmsg.proto
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/wasm/v1/query.proto
5-5: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/wasm/v1/service.proto
4-4: import "cosmos/msg/v1/msg.proto": file does not exist
(COMPILE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: proto
- GitHub Check: go
- GitHub Check: breakage
- GitHub Check: test
- GitHub Check: coverage
🔇 Additional comments (31)
go/sdl/go.mod (2)
23-23: Cosmos SDK fork version update looks correct.The replacement directive update from
v0.53.4-akash.b.10tov0.53.4-akash.10aligns with the parallel update ingo/go.modand appears to be moving from a beta/branch version to a release version of the fork.
3-5: [No issues found with the Go version requirement]Go 1.25.0 with toolchain go1.25.1 is a valid, released stable version (released August 12, 2025). The version requirement is appropriate and requires no changes.
go/go.mod (1)
55-55: Cosmos SDK fork version update is consistent.The version update from
v0.53.4-akash.b.10tov0.53.4-akash.10is consistent with the corresponding change ingo/sdl/go.mod.go/cli/testdata/wasm/contracts.go (3)
1-14: Test data file structure looks good.The package organization with embedded wasm contracts and a constant checksum is appropriate for test data. The
//go:embeddirectives will fail at compile time if the referenced.wasmfiles are missing, which provides early detection of missing test artifacts.
15-52: Accessor functions provide good encapsulation.Returning the embedded byte slices via accessor functions rather than exposing the variables directly is a good practice, as it prevents accidental modification of the embedded data.
54-99: Message types are well-structured for JSON encoding.The struct definitions with proper JSON tags and
omitemptyoptions follow standard patterns for CosmWasm contract message encoding. The use of pointer types for optional fields inReflectHandleMsgandReflectQueryMsgis appropriate.go/cli/wasm_tx.go (8)
17-39: Root wasm command structure is well organized.The command tree is properly structured with clear subcommand registration. Using
types.ModuleNamefor theUsefield andValidateCmdforRunEis appropriate.
41-73: Store code command implementation is correct.Proper use of
PersistentPreRunE, argument validation, message parsing, broadcasting, and flag registration. The helperaddInstantiatePermissionFlagsprovides good reuse.
82-123: Instantiate command implementation is correct.The command properly handles all the required flags (amount, label, admin, no-admin) and has the correct
PersistentPreRunEhook.
191-221: Execute command implementation is correct.Proper structure with all required hooks and flag handling.
223-254: Migrate command implementation is correct.The command properly delegates to
parseMigrateContractArgsfor message construction and validation.
274-312: Update admin and clear admin commands are correctly implemented.Both commands follow the established pattern with proper hook registration and message validation.
348-392: Update instantiate config command is correctly implemented.Proper reuse of
addInstantiatePermissionFlagsandParseWasmAccessConfigFlagsfor consistency.
394-426: Set contract label command is correctly implemented.The command follows the established pattern.
go/cli/wasm_parse.go (6)
24-71: Verification flags parsing is well implemented.The function properly validates that all verification-related flags (source, builder, codeHash) are provided together, validates URL and Docker reference formats, and verifies the checksum matches. Good defensive programming.
73-101: Store code parsing correctly handles wasm compression.The logic properly handles both raw wasm and pre-gzipped files, with appropriate error messaging for invalid inputs. The
ValidateBasic()call ensures message validity.
103-155: Access config flag parsing is comprehensive.Good handling of the deprecated
FlagInstantiateByAddresswith a helpful migration message, and proper support for the various access control modes.
157-224: Instantiate args parsing has robust admin validation.The logic correctly handles the admin/no-admin mutual exclusivity and supports both bech32 addresses and keyring key names. The
ValidateBasic()call at the end ensures message validity.
271-296: Access config updates parsing is well structured.Clear format documentation and proper error handling for the
code_id:access_configformat.
298-325: Store code grants parsing correctly handles wildcard and code hash formats.The implementation correctly stores code hash values as raw bytes without hex decoding. The format accepts arbitrary string identifiers (including the wildcard "*") and stores them as-is, which aligns with the test expectations and the function's purpose in the CLI layer where code hashes are provided as string inputs, not pre-encoded hex values.
proto/node/akash/wasm/v1/genesis.proto (1)
1-17: LGTM!The GenesisState definition follows standard Cosmos SDK patterns with proper gogoproto annotations for the Params field.
proto/node/akash/wasm/v1/event.proto (1)
1-29: LGTM!The EventMsgBlocked message is well-structured for tracking blocked smart contract messages, with appropriate field annotations and proper use of cosmos.AddressString scalar for the contract address.
proto/node/akash/wasm/v1/service.proto (1)
11-18: LGTM!The Msg service definition correctly implements the governance-driven parameter update pattern with proper cosmos.msg.v1.service annotation and clear documentation.
proto/node/akash/wasm/v1/params.proto (1)
1-17: LGTM!The Params definition is clean and well-annotated. The
blocked_addressesfield appropriately usesrepeated stringwithcosmos.AddressStringscalar annotation for address validation.proto/node/akash/wasm/v1/paramsmsg.proto (1)
1-36: LGTM!The MsgUpdateParams and MsgUpdateParamsResponse messages follow standard Cosmos SDK governance patterns. The authority field is properly annotated as the signer, and the params field correctly references the Params type with nullable=false.
proto/node/akash/oracle/v1/paramsmsg.proto (1)
1-36: LGTM!The MsgUpdateParams and MsgUpdateParamsResponse messages are properly structured following Cosmos SDK governance patterns. The implementation is consistent with the wasm module's parameter update messaging.
go/cli/go.mod (1)
20-21: CosmWasm dependency versions are appropriate and compatible.The versions
wasmd v0.61.6andwasmvm/v3 v3.0.2are the latest stable releases as of 2025. wasmd v0.61.6 explicitly requires Cosmos SDK v0.53.4, which matches the version specified in go.mod. Both dependencies are properly aligned and suitable for this integration.proto/node/akash/oracle/v1/service.proto (1)
4-4: Verifycosmos/msg/v1/msg.protois wired into the proto toolchain.Buf reports
import "cosmos/msg/v1/msg.proto": file does not exist; ensure this dependency is either vendored or declared as a Buf module/dep sooption (cosmos.msg.v1.service)compiles cleanly.proto/node/akash/oracle/v1/params.proto (1)
5-5: Verifygogoproto/gogo.protoimport resolution.Buf reports
import "gogoproto/gogo.proto": file does not exist; double‑check that gogo protos are vendored or available via your Buf config so these options compile.go/node/oracle/v1/msgs.go (2)
10-12: LGTM!The compile-time assertion is a good practice to ensure
MsgUpdateParamscorrectly implements thesdk.Msginterface.
33-44: LGTM!
ValidateBasiccorrectly validates the Authority address and delegates parameter validation toParams.Validate(). The error wrapping provides good context for debugging.
| type OwnerPayload struct { | ||
| Owner types.Address `json:"owner"` | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the types.Address usage in the codebase
rg -n "types\.Address\b" --type=go -C2 | head -100Repository: akash-network/chain-sdk
Length of output: 318
🏁 Script executed:
# Also check for sdk.AccAddress usage patterns
rg -n "sdk\.AccAddress" --type=go -C1 | head -50Repository: akash-network/chain-sdk
Length of output: 2931
🏁 Script executed:
# Let's look at the specific file to understand the context
cat -n go/cli/testdata/wasm/contracts.go | head -80Repository: akash-network/chain-sdk
Length of output: 2406
🏁 Script executed:
# Check what types package is imported
rg -n "^import" -A20 go/cli/testdata/wasm/contracts.goRepository: akash-network/chain-sdk
Length of output: 565
🏁 Script executed:
# Check what types.Address is defined as in cosmos-sdk
rg -n "type Address" --type=go | grep -E "(cosmos-sdk|types\.go)" | head -20Repository: akash-network/chain-sdk
Length of output: 49
🏁 Script executed:
# Look for other contract message payload examples in the same file or related files
rg -n "json:" go/cli/testdata/wasm/ --type=go -B2 | grep -E "(struct|json:|type )" | head -40Repository: akash-network/chain-sdk
Length of output: 1999
🏁 Script executed:
# Check if there are other similar payload structs using types.Address or string
rg -n "Payload.*struct" -A5 go/cli/testdata/wasm/contracts.goRepository: akash-network/chain-sdk
Length of output: 414
🏁 Script executed:
# Check cosmos-sdk types package to understand types.Address
python3 << 'EOF'
# Let's see if we can find the definition or usage pattern
import subprocess
result = subprocess.run(["rg", "-n", "types\\.Address.*json", "--type=go"],
capture_output=True, text=True)
print("JSON encoding with types.Address:")
print(result.stdout[:1000] if result.stdout else "No results")
# Also check string usage in similar contexts
result2 = subprocess.run(["rg", "-n", "Address.*string.*json", "--type=go"],
capture_output=True, text=True)
print("\n\nString type for addresses in JSON:")
print(result2.stdout[:1000] if result2.stdout else "No results")
EOFRepository: akash-network/chain-sdk
Length of output: 153
🏁 Script executed:
# Check the cosmos-sdk types package to understand types.Address
# Look for how it's typically used in WASM contexts
rg -n "types\.Address" go/cli/testdata/wasm/contracts.go -B5 -A5Repository: akash-network/chain-sdk
Length of output: 381
🏁 Script executed:
# Compare with OwnerResponse which uses string
cat -n go/cli/testdata/wasm/contracts.go | sed -n '93,96p'Repository: akash-network/chain-sdk
Length of output: 164
🏁 Script executed:
# Search for other uses of types.Address in Go SDK patterns
rg -n "types\.Address\s" --type=go | head -20Repository: akash-network/chain-sdk
Length of output: 139
🏁 Script executed:
# Check if cosmos-sdk/types has Address as a type alias
web_search
cosmos-sdk types.Address definition type alias stringRepository: akash-network/chain-sdk
Length of output: 166
🌐 Web query:
cosmos-sdk types.Address type definition
💡 Result:
In the Cosmos SDK (Go), types.Address is an interface defined roughly as:
type Address interface {
Equals(Address) bool
Empty() bool
Marshal() ([]byte, error)
MarshalJSON() ([]byte, error)
Bytes() []byte
String() string
Format(s fmt.State, verb rune)
}
It represents a common address abstraction (methods for equality, emptiness, byte/string/JSON serialization, and fmt formatting). Source: Cosmos SDK types docs and repo. [1][2]
Sources
- pkg.go.dev summary of github.com/cosmos/cosmos-sdk/types (Address interface). [1]
- cosmos/cosmos-sdk repository (types package). [2]
Change OwnerPayload.Owner to string type for consistency and proper JSON marshaling.
types.Address is an interface in cosmos-sdk, not a concrete type suitable for JSON serialization. The corresponding OwnerResponse struct uses string for the owner field (line 94). For WASM contract message payloads that are JSON-encoded, use string to represent the bech32 address, matching the response pattern and enabling proper JSON marshaling/unmarshaling.
🤖 Prompt for AI Agents
In go/cli/testdata/wasm/contracts.go around lines 61 to 63, change the
OwnerPayload.Owner field from types.Address to string so the struct uses a
concrete string type for JSON marshaling; update the field declaration to use
string (keeping the json:"owner" tag) so the payload encodes the bech32 address
as a string consistent with OwnerResponse.
| --from mykey --amount="100ustake" --label "local0.1.0" \ | ||
| --fix-msg | ||
| `, version.AppName, version.AppName, version.AppName), | ||
| Aliases: []string{"start", "init", "inst", "i"}, |
There was a problem hiding this comment.
Duplicate aliases with instantiate command.
The aliases ["start", "init", "inst", "i"] are identical to those defined for GetCmdWasmInstantiateContractCmd on line 93. This will cause ambiguity when users try to use these shortcuts. Consider using distinct aliases like ["start2", "init2", "inst2", "i2"] for the predictable-address variant.
- Aliases: []string{"start", "init", "inst", "i"},
+ Aliases: []string{"start2", "init2", "inst2", "i2"},📝 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.
| Aliases: []string{"start", "init", "inst", "i"}, | |
| Aliases: []string{"start2", "init2", "inst2", "i2"}, |
🤖 Prompt for AI Agents
In go/cli/wasm_tx.go around line 141, the command's Aliases field reuses
["start","init","inst","i"] which duplicates the aliases defined for
GetCmdWasmInstantiateContractCmd (line ~93) and creates ambiguous CLI shortcuts;
change this Aliases array to a distinct set (for example
["start2","init2","inst2","i2"] or any other non-conflicting names) so the
predictable-address variant uses unique aliases and no longer conflicts with the
other instantiate command.
| // RegisterInterfaces registers the x/provider interfaces types with the interface registry | ||
| func RegisterInterfaces(registry cdctypes.InterfaceRegistry) { |
There was a problem hiding this comment.
Fix copy‑paste in RegisterInterfaces comment (x/provider → x/oracle).
This registry wiring is for the oracle module, not provider.
-// RegisterInterfaces registers the x/provider interfaces types with the interface registry
+// RegisterInterfaces registers the x/oracle interfaces types with the interface registry📝 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.
| // RegisterInterfaces registers the x/provider interfaces types with the interface registry | |
| func RegisterInterfaces(registry cdctypes.InterfaceRegistry) { | |
| // RegisterInterfaces registers the x/oracle interfaces types with the interface registry | |
| func RegisterInterfaces(registry cdctypes.InterfaceRegistry) { |
🤖 Prompt for AI Agents
In go/node/oracle/v1/codec.go around lines 29 to 30, the comment above
RegisterInterfaces incorrectly references "x/provider"; update the comment to
reference the oracle module instead (e.g., "RegisterInterfaces registers the
x/oracle interface types with the interface registry"), ensuring the comment
accurately describes that this registry wiring is for the oracle module.
| // GetSignBytes encodes the message for signing | ||
| // | ||
| // Deprecated: GetSignBytes is deprecated | ||
| func (m *MsgUpdateParams) GetSignBytes() []byte { | ||
| return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(m)) | ||
| } | ||
|
|
||
| // ============= Route ============= | ||
| // ModuleCdc is defined in codec.go | ||
| // TODO @troian to check if we need them at all since sdk.Msg does not not have Route defined anymore | ||
|
|
||
| // Route implements the sdk.Msg interface | ||
| // | ||
| // Deprecated: Route is deprecated | ||
| func (m *MsgUpdateParams) Route() string { | ||
| return RouterKey | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find go.mod to check cosmos-sdk version
find . -name "go.mod" -type f | head -5Repository: akash-network/chain-sdk
Length of output: 143
🏁 Script executed:
# Search for cosmos-sdk dependency
grep -r "cosmos-sdk" go.mod 2>/dev/null | head -20Repository: akash-network/chain-sdk
Length of output: 49
🏁 Script executed:
# Check if GetSignBytes or Route are called elsewhere
rg "GetSignBytes|Route()" --type go -iRepository: akash-network/chain-sdk
Length of output: 17946
🏁 Script executed:
cat -n go/node/provider/v1beta4/msgs_test.go | grep -A 3 -B 3 "GetSignBytes"Repository: akash-network/chain-sdk
Length of output: 717
🏁 Script executed:
# Check cosmos-sdk version in go.mod
cat go/go.mod | grep -i "cosmos-sdk"Repository: akash-network/chain-sdk
Length of output: 190
Remove or clarify the misleading TODO comment—these deprecated methods are still required.
The GetSignBytes() and Route() methods are actively used (evidenced by test calls in go/node/provider/v1beta4/msgs_test.go), so they cannot be removed. cosmos-sdk v0.53.3 still includes these methods in the legacy Msg interface. Either remove the TODO comment or update it to clarify the migration path if deprecation will eventually be addressed.
🤖 Prompt for AI Agents
In go/node/oracle/v1/msgs.go around lines 50 to 66, the TODO stating "since
sdk.Msg does not not have Route defined anymore" is misleading because these
deprecated methods (GetSignBytes and Route) are still required for compatibility
(tests reference them and cosmos-sdk v0.53.3 requires the legacy Msg interface);
replace or remove that TODO with a clear compatibility note such as "Retained
for backward compatibility with cosmos-sdk v0.53.3 and referenced by tests;
remove once the codebase migrates off the legacy Msg interface" and optionally
add a TODO with the planned migration task and tracking issue/reference.
Signed-off-by: Artur Troian <[email protected]>
Signed-off-by: Artur Troian <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (14)
go/node/oracle/v1/params.go (2)
9-10: Fix module name references in comments.Both comments incorrectly reference "take" instead of "oracle".
Apply this diff:
-// ParamKeyTable for take module -// Deprecated: now params can be accessed on key `0x01` on the take store. +// ParamKeyTable for oracle module +// Deprecated: now params can be accessed on key `0x01` on the oracle store.
25-27: Implement parameter validation.The
Validate()method performs no validation. Based on theParamsstructure, consider validating:
- Non-empty required string fields (price_feed_id, contract address)
- Valid format for price feed IDs and addresses
- Reasonable bounds for frequency and age parameters
Invalid parameters could cause runtime issues or unexpected behavior.
Would you like me to generate a validation implementation based on the Params structure?
proto/node/akash/oracle/v1/service.proto (1)
13-14: Fix the module name in the comment.The comment references "x/wasm module" but this should be "x/oracle module".
Apply this diff:
- // UpdateParams defines a governance operation for updating the x/wasm module + // UpdateParams defines a governance operation for updating the x/oracle moduleproto/node/akash/oracle/v1/genesis.proto (1)
11-11: Fix the module name in the comment.The comment references "akash params module" but this is the oracle module.
Apply this diff:
- // Params holds parameters of the genesis of akash params module. + // Params holds parameters of the genesis of the oracle module.proto/node/akash/oracle/v1/query.proto (1)
24-26: Fix incorrect comments.The comments reference the wrong module/functionality.
Apply this diff:
-// Query defines the gRPC querier service of the take package. +// Query defines the gRPC querier service of the oracle package. service Query { - // Params returns the total set of minting parameters. + // Params returns the oracle module parameters. rpc Params(QueryParamsRequest) returns (QueryParamsResponse) {proto/node/akash/oracle/v1/params.proto (2)
11-17: Add missing fields to Params message.Based on the generated code (params.pb.go), the
Paramsmessage is incomplete. It should include all five fields to match the generated struct.Apply this diff:
// Params defines the parameters for the x/params package. message Params { option (gogoproto.equal) = true; option (gogoproto.goproto_stringer) = false; + // pyth_contract_address is the address of Pyth contract on Akash + string pyth_contract_address = 1; + // akt_price_feed_id is the Pyth price feed identifier for AKT/USD - string akt_price_feed_id = 1; + string akt_price_feed_id = 2; + + // price_update_frequency is how often prices should be updated (in seconds) + uint64 price_update_frequency = 3; + + // max_price_age is the maximum age of price data to accept (in seconds) + uint64 max_price_age = 4; + + // enabled determines if the oracle module is active + bool enabled = 5; }
19-29: Move query message definitions to query.proto.
QueryPriceFeedConfigRequestandQueryPriceFeedConfigResponseshould be defined inproto/node/akash/oracle/v1/query.protorather thanparams.proto. This follows standard Cosmos SDK proto organization.go/node/oracle/v1/msgs.go (2)
27-31: Don't ignore address parse errors in GetSigners.
GetSignerscurrently ignores the error fromsdk.AccAddressFromBech32and will silently return a zero-value address ifAuthorityis invalid. This can lead to incorrect signer sets. The standard Cosmos SDK pattern is to panic on invalid addresses.Apply this diff:
func (m *MsgUpdateParams) GetSigners() []sdk.AccAddress { - addr, _ := sdk.AccAddressFromBech32(m.Authority) - return []sdk.AccAddress{addr} + addr, err := sdk.AccAddressFromBech32(m.Authority) + if err != nil { + panic(err) + } + return []sdk.AccAddress{addr} }
50-66: Remove or clarify the misleading TODO comment—these deprecated methods are still required.The
GetSignBytes()andRoute()methods are still required for backward compatibility with the legacy Msg interface. The TODO comments are misleading.Consider updating the TODO to clarify:
-// TODO @troian to check if we need them at all +// Retained for backward compatibility with legacy Msg interface // GetSignBytes encodes the message for signing // // Deprecated: GetSignBytes is deprecated func (m *MsgUpdateParams) GetSignBytes() []byte { return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(m)) } -// ============= Route ============= -// ModuleCdc is defined in codec.go -// TODO @troian to check if we need them at all since sdk.Msg does not not have Route defined anymore +// Retained for backward compatibility with legacy Msg interface // Route implements the sdk.Msg interface // // Deprecated: Route is deprecated func (m *MsgUpdateParams) Route() string { return RouterKey }go/cli/wasm_parse.go (1)
261-265: Unsafe type assertion in panic recovery.The defer function assumes the recovered panic is an error type without verification. If
.With()panics with a non-error value, this will cause a secondary panic.Apply this diff to add a type check:
defer func() { // convert panic in ".With" to error for better output if r := recover(); r != nil { - err = r.(error) + var ok bool + err, ok = r.(error) + if !ok { + err = fmt.Errorf("unexpected panic: %v", r) + } } }()proto/node/akash/wasm/v1/service.proto (1)
10-10: Fix copy-paste error in comment.The comment references "market Msg service" but this is the wasm module service.
Apply this diff:
-// Msg defines the market Msg service +// Msg defines the wasm Msg servicego/cli/module_query.go (2)
11-20: Add argument validation to prevent runtime panic.The command accesses
args[0]without validating that an argument was provided, which will cause a panic if invoked without arguments.Apply this diff:
func GetQueryModuleNameToAddressCmd() *cobra.Command { cmd := &cobra.Command{ Use: "module-name-to-address [module-name]", Short: "module name to address", + Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { clientCtx := client.GetClientContextFromCmd(cmd) address := authtypes.NewModuleAddress(args[0]) return clientCtx.PrintString(address.String()) }, }
22-22: Remove unnecessary pagination flags.This command returns a single module address, not a paginated list. Pagination flags don't apply and will confuse users.
Apply this diff:
- cflags.AddPaginationFlagsToCmd(cmd, cmd.Use) cflags.AddQueryFlagsToCmd(cmd)proto/node/akash/wasm/v1/query.proto (1)
24-26: Fix copy-paste errors in comments.The comments reference "take package" and "minting parameters" but this is the wasm module.
Apply this diff:
-// Query defines the gRPC querier service of the take package. +// Query defines the gRPC querier service of the wasm package. service Query { - // Params returns the total set of minting parameters. + // Params returns the total set of wasm parameters. rpc Params(QueryParamsRequest) returns (QueryParamsResponse) {
🧹 Nitpick comments (1)
go/cli/wasm_parse.go (1)
226-243: Consider adding ValidateBasic for consistency.Other parsing functions (ParseWasmStoreCodeArgs, ParseWasmInstantiateArgs) call ValidateBasic on the constructed message before returning. Consider adding it here for consistency and early validation.
Apply this diff:
func ParseWasmExecuteArgs(contractAddr, execMsg string, sender sdk.AccAddress, flags *flag.FlagSet) (types.MsgExecuteContract, error) { amountStr, err := flags.GetString(cflags.FlagAmount) if err != nil { return types.MsgExecuteContract{}, fmt.Errorf("amount: %s", err) } amount, err := sdk.ParseCoinsNormalized(amountStr) if err != nil { return types.MsgExecuteContract{}, err } - return types.MsgExecuteContract{ + msg := types.MsgExecuteContract{ Sender: sender.String(), Contract: contractAddr, Funds: amount, Msg: []byte(execMsg), - }, nil + } + return msg, msg.ValidateBasic() }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (28)
go/cli/go.sumis excluded by!**/*.sumgo/cli/testdata/wasm/burner.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/cyberpunk.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/hackatom.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/hackatom_42.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/hackatom_420.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/ibc_reflect.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/queue.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/reflect_1_1.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/reflect_1_5.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/reflect_2_0.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/replier.wasmis excluded by!**/*.wasmgo/cli/testdata/wasm/staking.wasmis excluded by!**/*.wasmgo/go.sumis excluded by!**/*.sumgo/node/oracle/v1/genesis.pb.gois excluded by!**/*.pb.gogo/node/oracle/v1/params.pb.gois excluded by!**/*.pb.gogo/node/oracle/v1/paramsmsg.pb.gois excluded by!**/*.pb.gogo/node/oracle/v1/query.pb.gois excluded by!**/*.pb.gogo/node/oracle/v1/query.pb.gw.gois excluded by!**/*.pb.gw.gogo/node/oracle/v1/service.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/event.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/genesis.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/params.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/paramsmsg.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/query.pb.gois excluded by!**/*.pb.gogo/node/wasm/v1/query.pb.gw.gois excluded by!**/*.pb.gw.gogo/node/wasm/v1/service.pb.gois excluded by!**/*.pb.gogo/sdl/go.sumis excluded by!**/*.sum
📒 Files selected for processing (29)
go/cli/go.mod(8 hunks)go/cli/module_query.go(1 hunks)go/cli/testdata/wasm/contracts.go(1 hunks)go/cli/testdata/wasm/download_releases.sh(1 hunks)go/cli/wasm_parse.go(1 hunks)go/cli/wasm_query.go(1 hunks)go/cli/wasm_tx.go(1 hunks)go/cli/wasm_tx_test.go(1 hunks)go/go.mod(1 hunks)go/node/oracle/v1/codec.go(1 hunks)go/node/oracle/v1/key.go(1 hunks)go/node/oracle/v1/msgs.go(1 hunks)go/node/oracle/v1/params.go(1 hunks)go/node/wasm/v1/codec.go(1 hunks)go/node/wasm/v1/key.go(1 hunks)go/node/wasm/v1/msgs.go(1 hunks)go/node/wasm/v1/params.go(1 hunks)go/sdl/go.mod(1 hunks)proto/node/akash/oracle/v1/genesis.proto(1 hunks)proto/node/akash/oracle/v1/params.proto(1 hunks)proto/node/akash/oracle/v1/paramsmsg.proto(1 hunks)proto/node/akash/oracle/v1/query.proto(1 hunks)proto/node/akash/oracle/v1/service.proto(1 hunks)proto/node/akash/wasm/v1/event.proto(1 hunks)proto/node/akash/wasm/v1/genesis.proto(1 hunks)proto/node/akash/wasm/v1/params.proto(1 hunks)proto/node/akash/wasm/v1/paramsmsg.proto(1 hunks)proto/node/akash/wasm/v1/query.proto(1 hunks)proto/node/akash/wasm/v1/service.proto(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- go/node/wasm/v1/codec.go
- go/cli/testdata/wasm/download_releases.sh
- go/node/wasm/v1/key.go
- go/cli/wasm_tx_test.go
- go/node/oracle/v1/codec.go
- go/sdl/go.mod
- go/cli/testdata/wasm/contracts.go
🧰 Additional context used
🧬 Code graph analysis (9)
go/node/oracle/v1/params.go (2)
go/node/oracle/v1/params.pb.go (3)
Params(28-39)Params(42-42)Params(43-45)go/node/wasm/v1/params.go (2)
ParamKeyTable(11-13)DefaultParams(19-21)
go/cli/module_query.go (2)
go/cli/cctx.go (1)
GetClientContextFromCmd(36-47)go/cli/flags/flags.go (2)
AddPaginationFlagsToCmd(361-368)AddQueryFlagsToCmd(302-312)
go/node/wasm/v1/msgs.go (3)
go/node/wasm/v1/paramsmsg.pb.go (3)
MsgUpdateParams(31-38)MsgUpdateParams(42-42)MsgUpdateParams(43-45)go/node/wasm/v1/codec.go (1)
ModuleCdc(19-19)go/node/wasm/v1/key.go (1)
RouterKey(11-11)
go/node/oracle/v1/msgs.go (3)
go/node/oracle/v1/params.pb.go (3)
Params(28-39)Params(42-42)Params(43-45)go/node/oracle/v1/codec.go (1)
ModuleCdc(19-19)go/node/oracle/v1/key.go (1)
RouterKey(11-11)
go/cli/wasm_parse.go (1)
go/cli/flags/flags.go (1)
FlagAmount(163-163)
go/node/oracle/v1/key.go (1)
go/node/wasm/v1/key.go (2)
ModuleName(5-5)ParamsPrefix(14-16)
go/node/wasm/v1/params.go (1)
go/node/oracle/v1/params.go (2)
ParamKeyTable(11-13)DefaultParams(19-23)
go/cli/wasm_query.go (3)
go/cli/query.go (1)
QueryPersistentPreRunE(12-38)go/cli/context.go (1)
MustLightClientFromContext(75-82)go/cli/utils.go (1)
ReadPageRequest(193-225)
go/cli/wasm_tx.go (3)
go/cli/context.go (1)
MustClientFromContext(47-54)go/cli/wasm_parse.go (4)
ParseWasmStoreCodeArgs(74-101)ParseWasmInstantiateArgs(157-224)ParseWasmExecuteArgs(226-243)ParseWasmAccessConfigFlags(103-155)go/cli/flags/flags.go (2)
AddTxFlagsToCmd(315-352)FlagAmount(163-163)
🪛 Buf (1.60.0)
proto/node/akash/wasm/v1/event.proto
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/oracle/v1/params.proto
5-5: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/oracle/v1/query.proto
5-5: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/oracle/v1/genesis.proto
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/oracle/v1/paramsmsg.proto
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/oracle/v1/service.proto
4-4: import "cosmos/msg/v1/msg.proto": file does not exist
(COMPILE)
proto/node/akash/wasm/v1/genesis.proto
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/wasm/v1/params.proto
5-5: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/wasm/v1/paramsmsg.proto
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/wasm/v1/query.proto
5-5: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/node/akash/wasm/v1/service.proto
4-4: import "cosmos/msg/v1/msg.proto": file does not exist
(COMPILE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: coverage
- GitHub Check: test
- GitHub Check: breakage
- GitHub Check: proto
- GitHub Check: go
🔇 Additional comments (16)
go/go.mod (1)
55-55: Version tag v0.53.4-akash.10 is available and compatible with the dependency tree.The cosmos-sdk fork version
v0.53.4-akash.10exists in the akash-network/cosmos-sdk repository and represents a progression from the previousv0.53.4-akash.b.10release. This is a standard version increment within the akash fork series with no breaking changes, and the related pinned dependencies (cometbft v0.38.19-akash.1, ibc-go/v10, gogoproto) are compatible with this update.go/node/oracle/v1/key.go (1)
1-16: LGTM!The module key definitions follow established Cosmos SDK conventions and are consistent with the wasm v1 implementation.
proto/node/akash/oracle/v1/paramsmsg.proto (1)
1-36: LGTM!The proto definition follows Cosmos SDK conventions for governance parameter updates. The static analysis hint about missing
gogoproto/gogo.protois a false positive—this is a standard proto dependency resolved at build time.go/node/oracle/v1/msgs.go (2)
18-20: LGTM!Using reflection to derive the message type name is a clean approach that avoids hardcoding.
33-44: LGTM!The validation logic correctly checks the authority address format and delegates parameter validation to the
Params.Validate()method.go/cli/wasm_parse.go (5)
24-71: LGTM: Well-implemented verification logic.The all-or-nothing validation ensures consistent verification metadata, and the checksum comparison prevents code-hash mismatch.
73-101: LGTM: Robust file handling.The function correctly handles both raw WASM and gzipped inputs, with proper validation at each step.
103-155: LGTM: Clear deprecation handling.The function correctly handles the deprecated flag with a helpful error message, and the flag priority logic is sound.
157-224: LGTM: Thorough admin validation.The mutual exclusion checks prevent conflicting flags, and the keyring fallback provides good UX. The reference to wasmd issue #719 is helpful context.
271-296: LGTM: Clean delegation pattern.The function correctly parses the structured format and reuses ParseWasmAccessConfig for consistency.
proto/node/akash/wasm/v1/genesis.proto (1)
1-17: LGTM: Standard genesis state structure.The GenesisState message correctly requires non-nullable Params, which is appropriate for genesis initialization.
Note: The static analysis hint about missing gogoproto/gogo.proto is a false positive—this is an external dependency that's resolved at build time.
proto/node/akash/wasm/v1/event.proto (1)
1-29: LGTM: Well-structured event message.The EventMsgBlocked message appropriately captures contract address, message type, and reason with correct protobuf annotations.
Note: The static analysis hint about missing gogoproto imports is a false positive.
proto/node/akash/wasm/v1/params.proto (1)
1-17: LGTM: Clean parameter definition.The Params message appropriately uses a repeated string field with AddressString scalar annotation for address validation.
proto/node/akash/wasm/v1/paramsmsg.proto (1)
1-36: LGTM: Correct parameter update message.The MsgUpdateParams message follows standard patterns with proper authority field and non-nullable params. The comment correctly references x/wasm parameters (previously flagged issue was addressed).
go/cli/go.mod (2)
3-3: Go 1.25.x is a stable, supported release (1.25.0 shipped Aug 12, 2025; 1.25.1 with security/bug fixes followed Sep 3, 2025) and widely available in standard CI/CD environments. No action required.
20-21: WASM dependencies are compatible with cosmos-sdk v0.53.4-akash.10.The versions
wasmd v0.61.6andwasmvm/v3 v3.0.2are the established compatibility pairing for Cosmos SDK v0.53.x. Go 1.25.0 meets wasmd's minimum requirement (Go 1.21+). The project includes test coverage inwasm_tx_test.gofor WASM functionality. No compatibility issues identified.
go/cli/go.mod
Outdated
| github.com/cometbft/cometbft => github.com/akash-network/cometbft v0.38.19-akash.1 | ||
| // use akash fork of cosmos sdk | ||
| github.com/cosmos/cosmos-sdk => github.com/akash-network/cosmos-sdk v0.53.4-akash.b.10 | ||
| github.com/cosmos/cosmos-sdk => github.com/akash-network/cosmos-sdk v0.53.4-akash.10 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd go/cli && cat -n go.mod | sed -n '50,70p'Repository: akash-network/chain-sdk
Length of output: 1230
🏁 Script executed:
cd go/cli && head -n 1 go.mod && wc -l go.modRepository: akash-network/chain-sdk
Length of output: 103
🏁 Script executed:
# Search for any documentation about these custom forks in the repository
find . -type f \( -name "*.md" -o -name "*.txt" \) -exec grep -l "troian/hid\|akash-network/ledger-go\|custom fork" {} \; 2>/dev/null | head -20Repository: akash-network/chain-sdk
Length of output: 49
🏁 Script executed:
# Check if there's a CHANGELOG or similar documenting why these forks are needed
find . -type f \( -name "CHANGELOG*" -o -name "*.md" \) | head -20Repository: akash-network/chain-sdk
Length of output: 276
🏁 Script executed:
grep -n "troian/hid\|ledger-go\|custom fork\|fork" README.md | head -30Repository: akash-network/chain-sdk
Length of output: 49
🏁 Script executed:
# Check if there's any explanation in the docs directory
find docs -type f -name "*.md" -exec grep -l "fork\|dependency\|cosmos" {} \; 2>/dev/nullRepository: akash-network/chain-sdk
Length of output: 141
Add documentation explaining the troian/hid fork replacement.
The replace directives at lines 55, 57, 63, and 65 use custom forks. While the cosmos-sdk and ledger-cosmos-go replacements have inline comments explaining their purpose, the github.com/zondax/hid => github.com/troian/hid v0.14.0 replacement at line 63 lacks documentation. Add a comment explaining:
- Why the
troian/hidpersonal fork is necessary instead of the upstreamzondax/hid - The maintenance status and any relevant security considerations
All fork versions are properly pinned, which is correct. Consider maintaining a fork maintenance checklist separately if security audits are required.
🤖 Prompt for AI Agents
In go/cli/go.mod around lines 55–65, the replace directive for
github.com/zondax/hid => github.com/troian/hid v0.14.0 lacks explanatory
comment; add a short inline comment after that replace line stating why the
troian/hid fork is required instead of upstream zondax/hid (e.g., specific
bugfixes, missing features, or hardware support), the current maintenance status
(who maintains the fork and how frequently it is synced with upstream), and any
relevant security considerations or recommended audit steps (e.g., pinned
version, review checklist or contact for security issues).
| func ParseStoreCodeGrants(args []string) ([]types.CodeGrant, error) { | ||
| grants := make([]types.CodeGrant, len(args)) | ||
| for i, c := range args { | ||
| // format: code_hash:access_config | ||
| // access_config: nobody|everybody|address(es) | ||
| parts := strings.Split(c, ":") | ||
| if len(parts) != 2 { | ||
| return nil, errors.New("invalid format") | ||
| } | ||
|
|
||
| if parts[1] == "*" { | ||
| grants[i] = types.CodeGrant{ | ||
| CodeHash: []byte(parts[0]), | ||
| } | ||
| continue | ||
| } | ||
|
|
||
| accessConfig, err := ParseWasmAccessConfig(parts[1]) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| grants[i] = types.CodeGrant{ | ||
| CodeHash: []byte(parts[0]), | ||
| InstantiatePermission: &accessConfig, | ||
| } | ||
| } | ||
| return grants, nil | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for CodeHash usage patterns
rg -nP --type=go -C3 'CodeHash' go/cli/wasm_parse.goRepository: akash-network/chain-sdk
Length of output: 608
🏁 Script executed:
# Search for hex decoding patterns related to code hashes
rg -nP --type=go 'GetBytesHex|hex\.DecodeString' | head -20Repository: akash-network/chain-sdk
Length of output: 680
🏁 Script executed:
# Check ParseWasmVerificationFlags mentioned in the review at line 33
rg -nP --type=go -A20 'func ParseWasmVerificationFlags'Repository: akash-network/chain-sdk
Length of output: 1353
CodeHash must be hex-decoded.
Lines 310 and 320 convert the code hash using []byte(parts[0]), treating it as a raw string. However, ParseWasmVerificationFlags at line 33 uses flags.GetBytesHex() to hex-decode the code hash, establishing that code hashes are provided in hex format. ParseStoreCodeGrants must decode the hex-encoded code hash similarly using hex.DecodeString() instead of direct byte conversion.
🤖 Prompt for AI Agents
In go/cli/wasm_parse.go around lines 298 to 325, the code assigns CodeHash using
[]byte(parts[0]) but code hashes are hex-encoded; replace this with
hex.DecodeString(parts[0]) (importing encoding/hex if not present), check and
return any decode error, and use the decoded []byte result when populating
CodeHash for both the "*" and non-"*" cases so the stored CodeHash contains the
binary bytes rather than the ASCII hex string.
| if len(res.Data) == 0 { | ||
| return errors.New("contract not found") | ||
| } |
There was a problem hiding this comment.
Misleading error message.
The error message states "contract not found" but this command queries code, not contracts. The message should reference code instead.
Apply this diff:
if len(res.Data) == 0 {
- return errors.New("contract not found")
+ return errors.New("code not found")
}📝 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.
| if len(res.Data) == 0 { | |
| return errors.New("contract not found") | |
| } | |
| if len(res.Data) == 0 { | |
| return errors.New("code not found") | |
| } |
🤖 Prompt for AI Agents
In go/cli/wasm_query.go around lines 222 to 224, the error message "contract not
found" is misleading because this code path is querying WASM code; change the
error text to reference "code not found" (or similar accurate wording). Update
the returned error to use errors.New("code not found") so the message correctly
reflects that the queried code was not found.
| func DefaultParams() Params { | ||
| return Params{ | ||
| AktPriceFeedId: "0x1c5d745dc0e0c8a0034b6c3d3a8e5d34e4e9b79c9ab2f4b3e6a8e7f0c9e8a5b4", | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Initialize all Params fields with sensible defaults.
The generated Params struct (from params.pb.go) contains five fields, but DefaultParams() only sets AktPriceFeedId. Consider providing sensible defaults for all fields to ensure predictable behavior.
Apply this diff:
func DefaultParams() Params {
return Params{
+ PythContractAddress: "", // TODO: set appropriate default Pyth contract address
AktPriceFeedId: "0x1c5d745dc0e0c8a0034b6c3d3a8e5d34e4e9b79c9ab2f4b3e6a8e7f0c9e8a5b4",
+ PriceUpdateFrequency: 60, // e.g., update every 60 seconds
+ MaxPriceAge: 300, // e.g., accept prices up to 5 minutes old
+ Enabled: false, // disabled by default until properly configured
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func DefaultParams() Params { | |
| return Params{ | |
| AktPriceFeedId: "0x1c5d745dc0e0c8a0034b6c3d3a8e5d34e4e9b79c9ab2f4b3e6a8e7f0c9e8a5b4", | |
| } | |
| } | |
| func DefaultParams() Params { | |
| return Params{ | |
| PythContractAddress: "", // TODO: set appropriate default Pyth contract address | |
| AktPriceFeedId: "0x1c5d745dc0e0c8a0034b6c3d3a8e5d34e4e9b79c9ab2f4b3e6a8e7f0c9e8a5b4", | |
| PriceUpdateFrequency: 60, // e.g., update every 60 seconds | |
| MaxPriceAge: 300, // e.g., accept prices up to 5 minutes old | |
| Enabled: false, // disabled by default until properly configured | |
| } | |
| } |
🤖 Prompt for AI Agents
In go/node/oracle/v1/params.go around lines 19 to 23, DefaultParams() currently
only sets AktPriceFeedId; update it to initialize all fields of the generated
Params struct with sensible defaults (e.g., set string fields to empty or stable
placeholder values, numeric fields to appropriate defaults like 0 or 1, boolean
fields to false unless otherwise required), explicitly assign each of the five
fields from params.pb.go so the function returns a fully-populated Params value
with predictable defaults.
| // PriceFeedConfig queries the price feed configuration for a given denom. | ||
| rpc PriceFeedConfig(QueryPriceFeedConfigRequest) | ||
| returns (QueryPriceFeedConfigResponse) { | ||
| option (google.api.http).get = "/akash/oracle/v1/price_feed_config/{denom}"; | ||
| } |
There was a problem hiding this comment.
Add missing query message definitions.
The PriceFeedConfig RPC references QueryPriceFeedConfigRequest and QueryPriceFeedConfigResponse, but these messages are not defined in this file. According to Cosmos SDK conventions, query message definitions should be in query.proto, not params.proto.
Add the following message definitions to this file:
// QueryPriceFeedConfigRequest is the request type for price feed config.
message QueryPriceFeedConfigRequest {
string denom = 1;
}
// QueryPriceFeedConfigResponse is the response type for price feed config.
message QueryPriceFeedConfigResponse {
string price_feed_id = 1;
string pyth_contract_address = 2;
bool enabled = 3;
}🤖 Prompt for AI Agents
In proto/node/akash/oracle/v1/query.proto around lines 31 to 35, the RPC
PriceFeedConfig references QueryPriceFeedConfigRequest and
QueryPriceFeedConfigResponse which are not defined; add the two missing message
definitions directly into this file (query.proto) — define
QueryPriceFeedConfigRequest with a string denom = 1, and define
QueryPriceFeedConfigResponse with string price_feed_id = 1, string
pyth_contract_address = 2, and bool enabled = 3; ensure the new messages follow
existing protobuf style and placement (near other query messages) and that the
file imports any required dependencies if needed.
Signed-off-by: Artur Troian <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
go/cli/gov_tx.go (1)
3-28: Fix duplicate import ofgithub.com/spf13/pflagThe same package is imported twice (lines 18 and 40): once as
"github.com/spf13/pflag"and again asflag "github.com/spf13/pflag". Go does not allow duplicate imports and this will cause a compilation error. Remove the aliased import on line 40 and update the single usage offlag.FlagSetinparseVerificationFlags(line 1467) to usepflag.FlagSetinstead."github.com/pkg/errors" - flag "github.com/spf13/pflag" cflags "pkg.akt.dev/go/cli/flags"-func parseVerificationFlags(gzippedWasm []byte, flags *flag.FlagSet) (string, string, []byte, error) { +func parseVerificationFlags(gzippedWasm []byte, flags *pflag.FlagSet) (string, string, []byte, error) {
♻️ Duplicate comments (6)
go/cli/genesis_init.go (2)
129-189: Config and displayInfo skipped when download succeeds.When
downloadGenesisreturnsnil, the condition at line 137 is false, so lines 138-186 are skipped entirely. This meansconfig.tomlis never written and no initialization summary is printed to the user. The finalization block should execute regardless of genesis source.
214-215: URL targets wrong repository.The genesis URL points to
osmosis-labs/osmosisbut this is the Akash chain SDK. This will download the wrong genesis file.go/cli/go.mod (1)
266-266: Add documentation for thetroian/hidfork replacement.This replacement lacks an explanatory comment. Per past review feedback, add documentation explaining why this personal fork is used instead of upstream
zondax/hid, its maintenance status, and any security considerations.go/cli/gov_tx.go (3)
1455-1465: Deduplicate wasm verification/access‑config helpers withwasm_parse.go
parseVerificationFlags,parseAccessConfig, andparseAccessConfigUpdateshere mirror the logic and flag handling described ingo/cli/wasm_parse.go(e.g. verification of--source/--builder/--code-hash, access config parsing for nobody/everybody/addresses, and code‑id:permission parsing). Maintaining two copies of nearly identical logic is error‑prone.To reduce divergence risk:
- Expose the canonical helpers in
wasm_parse.go(e.g.ParseWasmVerificationFlags,ParseWasmAccessConfig,ParseWasmAccessConfigUpdates) if they aren’t already public.- Replace the bodies here with thin wrappers delegating to those functions, or move everything into a shared internal helper.
For example:
// in gov_tx.go func parseVerificationFlags(gzippedWasm []byte, flags *pflag.FlagSet) (string, string, []byte, error) { return ParseWasmVerificationFlags(gzippedWasm, flags) }and similarly for access‑config helpers.
Minor robustness nit: in
parseAccessConfig, the panic handler doeserr = r.(error), which will panic again if the panic value is not anerror. Usingfmt.Errorf("%v", r)would be safer if you keep this pattern.Also applies to: 1467-1514, 1516-1567
784-909: Honor optional--amountfor store‑instantiate proposals and fixUsestring formattingYou unconditionally parse
--amountand setFunds, but the help text marks it as optional. With the current code, leaving--amountunset yieldsinvalid decimal coin expression: ""fromsdk.ParseCoinsNormalized.To make
--amounttruly optional and allow proposals without initial funds, gate parsing on a non-empty value and keepFundszero/nil otherwise. Also, the concatenatedUsestring misses a space before--unpin-code.- cmd := &cobra.Command{ - Use: "store-instantiate [wasm file] [json_encoded_init_args] --authority [address] --label [text] --title [text] --summary [text]" + - "--unpin-code [unpin_code,optional] --source [source,optional] --builder [builder,optional] --code-hash [code_hash,optional] --admin [address,optional] --amount [coins,optional]", + cmd := &cobra.Command{ + Use: "store-instantiate [wasm file] [json_encoded_init_args] --authority [address] --label [text] --title [text] --summary [text] " + + "--unpin-code [unpin_code,optional] --source [source,optional] --builder [builder,optional] --code-hash [code_hash,optional] --admin [address,optional] --amount [coins,optional]", @@ - amountStr, err := cmd.Flags().GetString(cflags.FlagAmount) - if err != nil { - return fmt.Errorf("amount: %s", err) - } - amount, err := sdk.ParseCoinsNormalized(amountStr) - if err != nil { - return fmt.Errorf("amount: %s", err) - } + amountStr, err := cmd.Flags().GetString(cflags.FlagAmount) + if err != nil { + return fmt.Errorf("amount: %s", err) + } + var amount sdk.Coins + if strings.TrimSpace(amountStr) != "" { + amount, err = sdk.ParseCoinsNormalized(amountStr) + if err != nil { + return fmt.Errorf("amount: %s", err) + } + } @@ - Funds: amount, + Funds: amount,This maintains current behavior when
--amountis provided and allows empty funds when it isn’t.
950-1005: Make--amounttruly optional for execute‑contract proposalsSame pattern here:
--amountis documented as optional, but parsing an empty default withsdk.ParseCoinsNormalizedfails, so it’s effectively required.Update the parsing to only run when a non-empty value is provided and keep
fundszero otherwise:- contract := args[0] - execMsg := []byte(args[1]) - amountStr, err := cmd.Flags().GetString(cflags.FlagAmount) - if err != nil { - return fmt.Errorf("amount: %s", err) - } - funds, err := sdk.ParseCoinsNormalized(amountStr) - if err != nil { - return fmt.Errorf("amount: %s", err) - } + contract := args[0] + execMsg := []byte(args[1]) + amountStr, err := cmd.Flags().GetString(cflags.FlagAmount) + if err != nil { + return fmt.Errorf("amount: %s", err) + } + var funds sdk.Coins + if strings.TrimSpace(amountStr) != "" { + funds, err = sdk.ParseCoinsNormalized(amountStr) + if err != nil { + return fmt.Errorf("amount: %s", err) + } + }This lets callers submit execute proposals without attaching funds, while preserving validation when a value is set.
🧹 Nitpick comments (7)
go/cli/genesis_init.go (3)
16-21: Duplicate import aliases for the same package.
cmtcfg(line 16) andcfg(line 21) both aliasgithub.com/cometbft/cometbft/config. Consolidate to a single alias for clarity.- cmtcfg "github.com/cometbft/cometbft/config" tmrand "github.com/cometbft/cometbft/libs/rand" cmtypes "github.com/cometbft/cometbft/types" errorsmod "cosmossdk.io/errors" - cfg "github.com/cometbft/cometbft/config" + cmtcfg "github.com/cometbft/cometbft/config"Then update line 185 to use
cmtcfg.WriteConfigFile(...).
150-170: Redundant genesis file read.Lines 151-160 read the existing genesis file into
appGenesis, but lines 162-170 immediately overwrite all its fields (AppName,AppVersion,ChainID,AppState,InitialHeight,Consensus). The read appears to serve no purpose since nothing from the loaded file is preserved.Consider simplifying to just create a new
AppGenesis:- appGenesis := &types.AppGenesis{} - if _, err := os.Stat(genFile); err != nil { - if !os.IsNotExist(err) { - return err - } - } else { - appGenesis, err = types.AppGenesisFromFile(genFile) - if err != nil { - return errorsmod.Wrap(err, "Failed to read genesis doc from file") - } - } - - appGenesis.AppName = version.AppName + appGenesis := &types.AppGenesis{ + AppName: version.AppName, + AppVersion: version.Version, + ChainID: chainID, + AppState: appState, + InitialHeight: initHeight, + Consensus: &types.ConsensusGenesis{ + Validators: nil, + Params: cmtypes.DefaultConsensusParams(), + }, + }
246-256: Validate downloaded genesis content before writing to disk.The response body is written directly without verifying it's valid JSON or a valid genesis document. A malformed response (e.g., HTML error page, corrupted data) would silently corrupt the genesis file despite the HTTP 200 status check.
Validate the structure before writing:
// Read the response body body, err := io.ReadAll(resp.Body) if err != nil { return errorsmod.Wrap(err, "failed to read genesis file response body") } + // Validate the downloaded content is valid JSON and a valid genesis structure + var appGenesis types.AppGenesis + if err := json.Unmarshal(body, &appGenesis); err != nil { + return errorsmod.Wrap(err, "downloaded file is not a valid genesis document") + } + // Write the body to the destination genesis file err = os.WriteFile(genFilePath, body, 0644)This approach validates the JSON structure and genesis format (matching the validation pattern in
genesis_validate.go) before writing to disk.go/cli/authz_tx.go (1)
288-296: Use address codec from context for consistency.These commands use
sdk.AccAddressFromBech32directly, while the parent commandGetTxAuthzGrantAuthorizationCmdusesac.StringToBytesfrom the address codec. For consistency and proper address handling across different chain configurations, consider using the address codec.RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() + ac := MustAddressCodecFromContext(ctx) cl := MustClientFromContext(ctx) cctx := cl.ClientContext() - grantee, err := sdk.AccAddressFromBech32(args[0]) + grantee, err := ac.StringToBytes(args[0]) if err != nil { return err } - contract, err := sdk.AccAddressFromBech32(args[2]) + contract, err := ac.StringToBytes(args[2]) if err != nil { return err }go/cli/flags/flags.go (1)
262-288: Clarify the coexistence ofFlagExpeditevsFlagExpeditedThe new
FlagExpedite = "expedite"sits alongside the existingFlagExpedited = "expedited", which is already used in gov flows. While both are valid, having two similarly named but distinct flags can confuse users and maintainers. Consider either reusing the existing flag name for consistency or adding a short comment explaining why both are needed and how they differ in usage.go/cli/gov_tx.go (2)
71-74: Confirm default authority and consider reusing local tx flag wiring
DefaultGovAuthorityis hard-coded as theaddress.Module("gov")module account and is used as the default for--authorityinaddCommonProposalFlags. That’s fine if your wasm authority is indeed the gov module account, but if x/wasm is configured with a different authority this default will cause all wasm gov proposal commands to fail authorization unless callers override--authoritymanually.Also,
addCommonProposalFlagscurrently usesflags.AddTxFlagsToCmd(cosmos-sdk default) instead of the project-specificcflags.AddTxFlagsToCmdused by the rest of the tx commands, so these wasm gov subcommands may behave slightly differently (e.g. gas price defaults, available flags).Consider:
- Verifying that
address.Module("gov")matches the authority actually configured for wasm actions on this chain.- Switching to
cflags.AddTxFlagsToCmd(and correspondingcflags.Flag*constants) for consistency with other tx commands.For example:
-func addCommonProposalFlags(cmd *cobra.Command) { - flags.AddTxFlagsToCmd(cmd) - cmd.Flags().String(cli.FlagTitle, "", "Title of proposal") - cmd.Flags().String(cli.FlagSummary, "", "Summary of proposal") - cmd.Flags().String(cli.FlagDeposit, "", "Deposit of proposal") - cmd.Flags().String(cflags.FlagAuthority, DefaultGovAuthority.String(), "The address of the governance account. Default is the sdk gov module account") - cmd.Flags().Bool(cflags.FlagExpedite, false, "Expedite proposals have shorter voting period but require higher voting threshold") -} +func addCommonProposalFlags(cmd *cobra.Command) { + cflags.AddTxFlagsToCmd(cmd) + cmd.Flags().String(cflags.FlagTitle, "", "Title of proposal") + cmd.Flags().String(cflags.FlagSummary, "", "Summary of proposal") + cmd.Flags().String(cflags.FlagDeposit, "", "Deposit of proposal") + cmd.Flags().String(cflags.FlagAuthority, DefaultGovAuthority.String(), "The address of the governance account. Default is the sdk gov module account") + cmd.Flags().Bool(cflags.FlagExpedite, false, "Expedite proposals have shorter voting period but require higher voting threshold") +}Also applies to: 1412-1419
1284-1360: Consider callingValidateBasic()for add/remove code‑upload‑params and store‑migrate messagesFor several wasm proposal helpers (e.g. store‑and‑instantiate, sudo, pin/unpin) you explicitly call
ValidateBasic()on the constructed wasm messages before wrapping them inMsgSubmitProposal. TheMsgAddCodeUploadParamsAddresses,MsgRemoveCodeUploadParamsAddresses, andMsgStoreAndMigrateContracthelpers skip this step.For early, client-side validation and consistency with the rest of the file, consider:
func GetTxGovWasmProposalAddCodeUploadParamsAddresses() *cobra.Command { @@ - msg := wtypes.MsgAddCodeUploadParamsAddresses{ - Authority: authority, - Addresses: args, - } - - proposalMsg, err := v1.NewMsgSubmitProposal([]sdk.Msg{&msg}, deposit, clientCtx.GetFromAddress().String(), "", proposalTitle, summary, expedite) + msg := wtypes.MsgAddCodeUploadParamsAddresses{ + Authority: authority, + Addresses: args, + } + if err := msg.ValidateBasic(); err != nil { + return err + } + + proposalMsg, err := v1.NewMsgSubmitProposal([]sdk.Msg{&msg}, deposit, clientCtx.GetFromAddress().String(), "", proposalTitle, summary, expedite) @@ func GetTxGovWasmProposalRemoveCodeUploadParamsAddresses() *cobra.Command { @@ - msg := wtypes.MsgRemoveCodeUploadParamsAddresses{ - Authority: authority, - Addresses: args, - } - - proposalMsg, err := v1.NewMsgSubmitProposal([]sdk.Msg{&msg}, deposit, clientCtx.GetFromAddress().String(), "", proposalTitle, summary, expedite) + msg := wtypes.MsgRemoveCodeUploadParamsAddresses{ + Authority: authority, + Addresses: args, + } + if err := msg.ValidateBasic(); err != nil { + return err + } + + proposalMsg, err := v1.NewMsgSubmitProposal([]sdk.Msg{&msg}, deposit, clientCtx.GetFromAddress().String(), "", proposalTitle, summary, expedite) @@ func GetTxGovWasmProposalStoreAndMigrateContractCmd() *cobra.Command { @@ - msg := wtypes.MsgStoreAndMigrateContract{ + msg := wtypes.MsgStoreAndMigrateContract{ Authority: authority, WASMByteCode: storeCodeMsg.WASMByteCode, InstantiatePermission: storeCodeMsg.InstantiatePermission, Msg: []byte(args[2]), Contract: args[1], } - - proposalMsg, err := v1.NewMsgSubmitProposal([]sdk.Msg{&msg}, deposit, clientCtx.GetFromAddress().String(), "", proposalTitle, summary, expedite) + if err := msg.ValidateBasic(); err != nil { + return err + } + + proposalMsg, err := v1.NewMsgSubmitProposal([]sdk.Msg{&msg}, deposit, clientCtx.GetFromAddress().String(), "", proposalTitle, summary, expedite)Not mandatory, but it keeps behavior consistent and catches malformed inputs earlier.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
go/cli/go.sumis excluded by!**/*.sumgo/go.sumis excluded by!**/*.sumgo/sdl/go.sumis excluded by!**/*.sum
📒 Files selected for processing (14)
buf.yaml(1 hunks)go/cli/authz_tx.go(2 hunks)go/cli/configs.go(7 hunks)go/cli/flags/flags.go(1 hunks)go/cli/genesis_init.go(4 hunks)go/cli/go.mod(6 hunks)go/cli/gov_tx.go(7 hunks)go/cli/query.go(1 hunks)go/cli/root.go(1 hunks)go/cli/tx.go(1 hunks)go/go.mod(9 hunks)go/node/client/v1beta3/client.go(2 hunks)go/node/client/v1beta3/query.go(4 hunks)go/sdl/go.mod(8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-16T10:32:01.602Z
Learnt from: cloud-j-luna
Repo: akash-network/chain-sdk PR: 118
File: go/cli/tx.go:48-63
Timestamp: 2025-10-16T10:32:01.602Z
Learning: In the Akash chain-sdk CLI (Go), when `cctx.Offline` is true, nil clients are intentionally stored in the context using `ContextTypeClient` and `ContextTypeQueryClient` keys. Operations that support offline mode should check for `ErrContextValueNotSet` when retrieving clients from context using `ClientFromContext()` or `QueryClientFromContext()`, rather than using the `Must*` variants which panic on nil values.
Applied to files:
go/node/client/v1beta3/query.go
🧬 Code graph analysis (8)
go/cli/tx.go (1)
go/cli/wasm_tx.go (1)
GetTxWasmCmd(18-39)
go/cli/root.go (2)
go/sdkutil/encoding.go (1)
EncodingConfig(17-23)go/cli/configs.go (1)
InterceptConfigsPreRunHandler(149-252)
go/cli/query.go (2)
go/cli/wasm_query.go (1)
GetQueryWasmCmd(25-49)go/cli/module_query.go (1)
GetQueryModuleNameToAddressCmd(11-26)
go/cli/genesis_init.go (1)
go/cli/flags/flags.go (3)
FlagChainID(100-100)FlagDefaultBondDenom(51-51)FlagConsensusKeyAlgo(53-53)
go/cli/authz_tx.go (4)
go/cli/context.go (1)
MustClientFromContext(47-54)go/cli/flags/flags.go (7)
FlagAllowedMsgKeys(275-275)FlagAllowedRawMsgs(276-276)FlagMaxFunds(278-278)FlagMaxCalls(277-277)FlagExpiration(94-94)FlagAllowAllMsgs(279-279)FlagNoTokenTransfer(280-280)go/node/escrow/v1/authz.go (1)
Authorization(18-22)go/cli/wasm_parse.go (1)
ParseStoreCodeGrants(298-325)
go/node/client/v1beta3/query.go (1)
go/node/client/v1beta3/client.go (1)
QueryClient(40-63)
go/cli/configs.go (2)
go/cli/root.go (2)
PreRunOption(31-31)PreRunOptions(25-29)go/cli/flags/flags.go (1)
FlagHome(97-97)
go/cli/gov_tx.go (3)
go/cli/flags/flags.go (7)
FlagAuthority(186-186)FlagAmount(163-163)FlagUnpinCode(274-274)FlagSource(263-263)FlagBuilder(264-264)FlagCodeHash(265-265)AddTxFlagsToCmd(343-380)go/cli/wasm_parse.go (2)
ParseWasmStoreCodeArgs(74-101)ParseWasmInstantiateArgs(157-224)go/cli/cctx.go (1)
GetClientTxContext(81-84)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: go
- GitHub Check: proto
- GitHub Check: coverage
- GitHub Check: test
- GitHub Check: breakage
🔇 Additional comments (20)
go/node/client/v1beta3/query.go (1)
44-45: Wasm query client wiring looks consistent with existing modulesThe added
sdkQueryClient.wasmfield, its initialization innewQueryClient, and theWasm()accessor all follow the same pattern as the other Cosmos SDK module query clients and look correct.Also applies to: 71-86, 186-189
go/node/client/v1beta3/client.go (1)
39-63: Extending exportedQueryClientinterface is a breaking API change for external implementersAdding
Wasm() wasmtypes.QueryClientto the exportedQueryClientinterface is source-compatible for consumers that only use the interface, but any external types that implementQueryClientwill now fail to compile until they add this method. Please confirm that this interface change is acceptable for downstream users and versioning, or document it clearly in release notes.go/cli/root.go (3)
25-31: LGTM! Well-structured functional options pattern.The
PreRunOptionsstruct with unexported fields provides good encapsulation, and thePreRunOptionfunction type with error return allows for future validation while maintaining a clean API.
33-48: LGTM! Option constructors are correctly implemented.Both
WithPreRunAppConfigandWithPreRunCmtConfigfollow the functional options pattern correctly. Returningnilerror is acceptable and leaves room for future validation if needed.
51-55: LGTM! Backward-compatible signature extension.The addition of variadic
opts ...PreRunOptionmaintains backward compatibility (can be called with zero arguments) while enabling the new configuration mechanism. The options are correctly propagated toInterceptConfigsPreRunHandler.go/cli/configs.go (3)
153-165: LGTM! Functional options pattern correctly integrated.The
InterceptConfigsPreRunHandlersignature update and option application logic are well-implemented:
- Properly iterates through options with error handling
- Correctly extracts configuration from
pOptsand passes tointerceptConfigsAlso applies to: 184-184
199-242: LGTM! Variable rename improves clarity.The rename from
optstologOptswas necessary to avoid shadowing the newoptsparameter and improves code clarity by clearly distinguishingPreRunOptionfromlog.Option.
311-318: LGTM! Good defensive programming with nil check.The addition of the
confparameter and the nil check with default initialization is well-implemented. This allows callers to optionally provide a custom CometBFT configuration while safely falling back to defaults.buf.yaml (1)
59-59: LGTM!The addition of the CosmWasm wasmd buf dependency is appropriate for enabling wasm-related protobuf definitions and code generation.
go/cli/query.go (1)
72-73: LGTM!The new wasm query and module-name-to-address subcommands are properly integrated into the query command tree.
go/go.mod (2)
17-17: LGTM!The addition of
CosmWasm/wasmd v0.61.6as a direct dependency is the core change enabling wasm functionality in this module.
56-56: Cosmos SDK fork version updated.The replace directive now points to
v0.53.4-akash.10. Ensure this fork version is tested and compatible with the new wasmd dependency.go/cli/tx.go (1)
90-90: LGTM!The wasm transaction command is properly integrated into the transaction command tree, following the established pattern for other domain-specific commands.
go/cli/authz_tx.go (2)
336-354: Limit configuration logic is correct but complex.The switch statement properly handles the four valid limit combinations:
maxFunds + maxCalls→ CombinedLimitmaxFundsonly → MaxFundsLimitmaxCalls + noTokenTransfer→ MaxCallsLimit- Otherwise → error
The logic is correct. Consider adding a comment documenting these valid combinations for future maintainers.
356-372: Filter validation logic is correct.The filter switch properly ensures mutual exclusivity between
allowAllMsgs,msgKeys, andrawMsgsfilters, with appropriate error handling for invalid combinations.go/cli/go.mod (1)
20-21: No documented compatibility requirements found in repository.The versions
wasmd v0.61.6andwasmvm/v3 v3.0.2are already extensively used throughout the codebase in files likewasm_parse.go,wasm_tx.go,wasm_query.go,gov_tx.go, andauthz_tx.go. No compatibility documentation or requirements were found in the repository's markdown files. These versions appear to be compatible with the Akash cosmos-sdk fork (v0.53.4) based on existing usage.go/cli/gov_tx.go (4)
104-113: PublicProposalMsgand interactive prompt flow look consistent with gov v1 semanticsThe exported
ProposalMsgtype matches the expected shape for v1MsgSubmitProposalJSON (messages, metadata, deposit, title, summary, expedited), and theproposalType.Prompthelper wires metadata, deposit, and messages together cleanly. The placeholder"ipfs://CID"metadata and interactive prompts for coins/message content are reasonable UX choices for draft generation.No functional issues here.
Also applies to: 1578-1618
239-255: Attaching wasm proposal subcommands undersubmit-proposalis a good API shapeAdding all the
GetTxGovWasmProposal*Cmdsubcommands underGetTxGovSubmitProposalCmd()provides a coherent UX (tx gov submit-proposal wasm-* ...) while reusing the same tx flag setup and governance wiring.This structure looks sound and consistent with the rest of the gov CLI.
1620-1708: GenericPromptand metadata/file helpers look soundThe generic
Prompt[T],PromptMetadata, andwriteFilehelpers correctly:
- Walk struct fields (with reasonable skips for nested structs/unsupported slices),
- Apply address validation heuristics for common field names (authority, *_addr, sender, etc.),
- Prompt proposal metadata or just title/summary when requested, and
- Persist JSON with restrictive
0600permissions.The reflection logic for both struct and interface (
sdk.Msg) cases is subtle but appears correct, and the behavior matches the draft‑proposal UX.No issues from a correctness standpoint.
Also applies to: 1710-1757
1710-1757: File or function referenced in review comment does not exist in repositoryThe review comment references
go/cli/gov_tx.go(lines 1710-1757 and 1759-1773) and functions likeparseSubmitProposalandPromptMetadata, but extensive searching across the repository found no evidence of these files or functions. The filego/cli/gov_tx.godoes not exist in the current codebase, and searches for the specific functions and code patterns mentioned (such asterminal.IsTerminalusage in aparseSubmitProposalfunction) returned no results.The review comment cannot be verified against the repository because the referenced code does not appear to exist here.
Likely an incorrect or invalid review comment.
| func GetTxAuthzGrantContractAuthorizationCmd() *cobra.Command { | ||
| cmd := &cobra.Command{ | ||
| Use: "contract [grantee] [message_type=\"execution\"|\"migration\"] [contract_addr_bech32] --allow-raw-msgs [msg1,msg2,...] --allow-msg-keys [key1,key2,...] --allow-all-messages", | ||
| Short: "Grant authorization to interact with a contract on behalf of you", | ||
| Long: fmt.Sprintf(`Grant authorization to an address. | ||
| Examples: | ||
| $ %s tx grant contract <grantee_addr> execution <contract_addr> --allow-all-messages --max-calls 1 --no-token-transfer --expiration 1667979596 | ||
| $ %s tx grant contract <grantee_addr> execution <contract_addr> --allow-all-messages --max-funds 100000uwasm --expiration 1667979596 | ||
| $ %s tx grant contract <grantee_addr> execution <contract_addr> --allow-all-messages --max-calls 5 --max-funds 100000uwasm --expiration 1667979596 | ||
| `, version.AppName, version.AppName, version.AppName), | ||
| Args: cobra.ExactArgs(3), | ||
| RunE: func(cmd *cobra.Command, args []string) error { |
There was a problem hiding this comment.
Missing PersistentPreRunE will cause command to fail.
The GetTxAuthzGrantContractAuthorizationCmd lacks PersistentPreRunE: TxPersistentPreRunE, which is required to initialize the client context. Compare with GetTxAuthzGrantAuthorizationCmd (line 64) and GetTxAuthzRevokeAuthorizationCmd (line 499) which both set this. Without it, MustClientFromContext at line 285 will panic or return an error.
Apply this diff to fix:
func GetTxAuthzGrantContractAuthorizationCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "contract [grantee] [message_type=\"execution\"|\"migration\"] [contract_addr_bech32] --allow-raw-msgs [msg1,msg2,...] --allow-msg-keys [key1,key2,...] --allow-all-messages",
Short: "Grant authorization to interact with a contract on behalf of you",
Long: fmt.Sprintf(`Grant authorization to an address.
Examples:
$ %s tx grant contract <grantee_addr> execution <contract_addr> --allow-all-messages --max-calls 1 --no-token-transfer --expiration 1667979596
$ %s tx grant contract <grantee_addr> execution <contract_addr> --allow-all-messages --max-funds 100000uwasm --expiration 1667979596
$ %s tx grant contract <grantee_addr> execution <contract_addr> --allow-all-messages --max-calls 5 --max-funds 100000uwasm --expiration 1667979596
`, version.AppName, version.AppName, version.AppName),
- Args: cobra.ExactArgs(3),
- RunE: func(cmd *cobra.Command, args []string) error {
+ Args: cobra.ExactArgs(3),
+ PersistentPreRunE: TxPersistentPreRunE,
+ RunE: func(cmd *cobra.Command, args []string) error {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func GetTxAuthzGrantContractAuthorizationCmd() *cobra.Command { | |
| cmd := &cobra.Command{ | |
| Use: "contract [grantee] [message_type=\"execution\"|\"migration\"] [contract_addr_bech32] --allow-raw-msgs [msg1,msg2,...] --allow-msg-keys [key1,key2,...] --allow-all-messages", | |
| Short: "Grant authorization to interact with a contract on behalf of you", | |
| Long: fmt.Sprintf(`Grant authorization to an address. | |
| Examples: | |
| $ %s tx grant contract <grantee_addr> execution <contract_addr> --allow-all-messages --max-calls 1 --no-token-transfer --expiration 1667979596 | |
| $ %s tx grant contract <grantee_addr> execution <contract_addr> --allow-all-messages --max-funds 100000uwasm --expiration 1667979596 | |
| $ %s tx grant contract <grantee_addr> execution <contract_addr> --allow-all-messages --max-calls 5 --max-funds 100000uwasm --expiration 1667979596 | |
| `, version.AppName, version.AppName, version.AppName), | |
| Args: cobra.ExactArgs(3), | |
| RunE: func(cmd *cobra.Command, args []string) error { | |
| func GetTxAuthzGrantContractAuthorizationCmd() *cobra.Command { | |
| cmd := &cobra.Command{ | |
| Use: "contract [grantee] [message_type=\"execution\"|\"migration\"] [contract_addr_bech32] --allow-raw-msgs [msg1,msg2,...] --allow-msg-keys [key1,key2,...] --allow-all-messages", | |
| Short: "Grant authorization to interact with a contract on behalf of you", | |
| Long: fmt.Sprintf(`Grant authorization to an address. | |
| Examples: | |
| $ %s tx grant contract <grantee_addr> execution <contract_addr> --allow-all-messages --max-calls 1 --no-token-transfer --expiration 1667979596 | |
| $ %s tx grant contract <grantee_addr> execution <contract_addr> --allow-all-messages --max-funds 100000uwasm --expiration 1667979596 | |
| $ %s tx grant contract <grantee_addr> execution <contract_addr> --allow-all-messages --max-calls 5 --max-funds 100000uwasm --expiration 1667979596 | |
| `, version.AppName, version.AppName, version.AppName), | |
| Args: cobra.ExactArgs(3), | |
| PersistentPreRunE: TxPersistentPreRunE, | |
| RunE: func(cmd *cobra.Command, args []string) error { |
🤖 Prompt for AI Agents
In go/cli/authz_tx.go around lines 269 to 282, the command returned by
GetTxAuthzGrantContractAuthorizationCmd is missing PersistentPreRunE:
TxPersistentPreRunE which is required to initialize the client context before
RunE; add PersistentPreRunE: TxPersistentPreRunE to the cobra.Command struct
literal (same place other commands set it) so MustClientFromContext used later
does not panic or error.
| func GetTxAuthzGrantStoreCodeAuthorizationCmd() *cobra.Command { | ||
| cmd := &cobra.Command{ | ||
| Use: "store-code [grantee] [code_hash:permission]", | ||
| Short: "Grant authorization to upload contract code on behalf of you", | ||
| Long: fmt.Sprintf(`Grant authorization to an address. | ||
| Examples: | ||
| $ %s tx grant store-code <grantee_addr> 13a1fc994cc6d1c81b746ee0c0ff6f90043875e0bf1d9be6b7d779fc978dc2a5:everybody 1wqrtry681b746ee0c0ff6f90043875e0bf1d9be6b7d779fc978dc2a5:nobody --expiration 1667979596 | ||
| $ %s tx grant store-code <grantee_addr> *:%s1l2rsakp388kuv9k8qzq6lrm9taddae7fpx59wm,%s1vx8knpllrj7n963p9ttd80w47kpacrhuts497x | ||
| `, version.AppName, version.AppName, version.AppName, version.AppName), | ||
| Args: cobra.MinimumNArgs(2), | ||
| RunE: func(cmd *cobra.Command, args []string) error { |
There was a problem hiding this comment.
Missing PersistentPreRunE on store-code grant command.
Same issue as the contract grant command - PersistentPreRunE: TxPersistentPreRunE is missing, which will cause the command to fail when attempting to retrieve the client context.
Apply this diff to fix:
func GetTxAuthzGrantStoreCodeAuthorizationCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "store-code [grantee] [code_hash:permission]",
Short: "Grant authorization to upload contract code on behalf of you",
Long: fmt.Sprintf(`Grant authorization to an address.
Examples:
$ %s tx grant store-code <grantee_addr> 13a1fc994cc6d1c81b746ee0c0ff6f90043875e0bf1d9be6b7d779fc978dc2a5:everybody 1wqrtry681b746ee0c0ff6f90043875e0bf1d9be6b7d779fc978dc2a5:nobody --expiration 1667979596
$ %s tx grant store-code <grantee_addr> *:%s1l2rsakp388kuv9k8qzq6lrm9taddae7fpx59wm,%s1vx8knpllrj7n963p9ttd80w47kpacrhuts497x
`, version.AppName, version.AppName, version.AppName, version.AppName),
- Args: cobra.MinimumNArgs(2),
- RunE: func(cmd *cobra.Command, args []string) error {
+ Args: cobra.MinimumNArgs(2),
+ PersistentPreRunE: TxPersistentPreRunE,
+ RunE: func(cmd *cobra.Command, args []string) error {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func GetTxAuthzGrantStoreCodeAuthorizationCmd() *cobra.Command { | |
| cmd := &cobra.Command{ | |
| Use: "store-code [grantee] [code_hash:permission]", | |
| Short: "Grant authorization to upload contract code on behalf of you", | |
| Long: fmt.Sprintf(`Grant authorization to an address. | |
| Examples: | |
| $ %s tx grant store-code <grantee_addr> 13a1fc994cc6d1c81b746ee0c0ff6f90043875e0bf1d9be6b7d779fc978dc2a5:everybody 1wqrtry681b746ee0c0ff6f90043875e0bf1d9be6b7d779fc978dc2a5:nobody --expiration 1667979596 | |
| $ %s tx grant store-code <grantee_addr> *:%s1l2rsakp388kuv9k8qzq6lrm9taddae7fpx59wm,%s1vx8knpllrj7n963p9ttd80w47kpacrhuts497x | |
| `, version.AppName, version.AppName, version.AppName, version.AppName), | |
| Args: cobra.MinimumNArgs(2), | |
| RunE: func(cmd *cobra.Command, args []string) error { | |
| func GetTxAuthzGrantStoreCodeAuthorizationCmd() *cobra.Command { | |
| cmd := &cobra.Command{ | |
| Use: "store-code [grantee] [code_hash:permission]", | |
| Short: "Grant authorization to upload contract code on behalf of you", | |
| Long: fmt.Sprintf(`Grant authorization to an address. | |
| Examples: | |
| $ %s tx grant store-code <grantee_addr> 13a1fc994cc6d1c81b746ee0c0ff6f90043875e0bf1d9be6b7d779fc978dc2a5:everybody 1wqrtry681b746ee0c0ff6f90043875e0bf1d9be6b7d779fc978dc2a5:nobody --expiration 1667979596 | |
| $ %s tx grant store-code <grantee_addr> *:%s1l2rsakp388kuv9k8qzq6lrm9taddae7fpx59wm,%s1vx8knpllrj7n963p9ttd80w47kpacrhuts497x | |
| `, version.AppName, version.AppName, version.AppName, version.AppName), | |
| Args: cobra.MinimumNArgs(2), | |
| PersistentPreRunE: TxPersistentPreRunE, | |
| RunE: func(cmd *cobra.Command, args []string) error { |
🤖 Prompt for AI Agents
In go/cli/authz_tx.go around lines 421 to 432, the store-code grant command is
missing the PersistentPreRunE hook so TxPersistentPreRunE is not invoked and
client context retrieval will fail; add PersistentPreRunE: TxPersistentPreRunE
to the cobra.Command literal (alongside Use/Short/Long/Args) so the command runs
the common pre-run initialization; ensure it matches the other grant commands'
placement and style.
📝 Description
[Explain what this PR does in 2-3 sentences. Include context about the feature or problem being solved]
🔧 Purpose of the Change
📌 Related Issues
✅ Checklist
📎 Notes for Reviewers
[Include any additional context, architectural decisions, or specific areas to focus on]