wardend: remove depinject #1653
Conversation
Pitasi
force-pushed
the
remove-depinject
branch
from
c77cbc5 to
ebf7caa
Compare
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Pitasi
force-pushed
the
remove-depinject
branch
from
ebf7caa to
6eb7335
Compare
Pitasi
force-pushed
the
remove-depinject
branch
5 times, most recently
from
c06fd62 to
62bf372
Compare
|
The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).
|
Pitasi
force-pushed
the
remove-depinject
branch
from
62bf372 to
ecd3266
Compare
This comment has been minimized.
This comment has been minimized.
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughRemoved DI-based CLI and many legacy app wiring files; introduced an explicit CLI bootstrap using a temporary in-memory app encoding, a new NewApp ModuleManager-style constructor and antecedent routing for EVM/sched/Cosmos, bank-aware precompiles, testnet tooling, consolidated Wardend config and EVM app options, widespread dependency upgrades, and multiple test/util deletions. Changes
Files marked with * were deleted or had their contents removed. Sequence Diagram(s)%%{init: {"theme": "base", "themeVariables": {"primaryColor":"#0B6E99","noteBkg":"#F2F8FB","actorBorder":"#0B6E99"}}}%%
sequenceDiagram
autonumber
actor User
participant CLI as wardend CLI
participant TempApp as tempApp (encoding)
participant Viper as Viper (config)
participant App as newApp
participant Comet as CometBFT
User->>CLI: invoke wardend <command>
CLI->>TempApp: construct temp app to derive encodingConfig
CLI->>Viper: read client & app config (home, chain-id)
Viper-->>CLI: clientCtx & templates
CLI->>App: newApp(logger, db, appOpts)
App-->>CLI: server.Application instance
CLI->>Comet: serve/start/status/genesis/tx/query
%%{init: {"theme": "base", "themeVariables": {"primaryColor":"#0B6E99","noteBkg":"#F2F8FB"}}}%%
sequenceDiagram
autonumber
participant BaseApp as BaseApp
participant Router as Ante Router
participant Cosmos as Cosmos Ante Chain
participant EVM as EVM Mono Ante
participant Sched as Sched Ante Chain
participant Tx as Transaction
BaseApp->>Router: deliverTx/checkTx(Tx)
alt Tx has extension options
Router->>Router: inspect first extension option TypeUrl
alt sched callbacks
Router->>Sched: execute sched ante
else ethereum tx extension
Router->>EVM: execute mono-EVM ante
else dynamic fee ext
Router->>Cosmos: execute cosmos ante
else unknown ext
Router-->>BaseApp: return ErrUnknownExtensionOptions
end
else no extension options
Router->>Cosmos: execute cosmos ante
end
Router-->>BaseApp: return (ctx, err)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related PRs
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
go.mod (1)
19-21: Security: upgrade gin to ≥ v1.7.7 to address GHSA-h395-qcrw-5vmqThe GitHub advisory GHSA-h395-qcrw-5vmq (CVE-2020-28483) affects all github.com/gin-gonic/gin versions below 1.7.7; v1.7.0 remains vulnerable and does not include the patch (osv.dev, miggo.io).
Please bump the module requirement in go.mod to at least v1.7.7—or better yet, to the latest stable release—to fully remediate this issue.• File: go.mod (lines 19–21)
Update the gin replacement line as follows:- github.com/gin-gonic/gin => github.com/gin-gonic/gin v1.7.0 + github.com/gin-gonic/gin => github.com/gin-gonic/gin v1.7.7warden/app/export.go (2)
61-71: Allowed-addresses map uses non-canonical keys; may jail allowed validatorsYou validate each bech32 address but store the original string as the map key, later checking with addr.String() (canonical form). If inputs vary in case/format, lookups fail and validators get jailed unintentionally.
Apply:
-allowedAddrsMap := make(map[string]bool) +allowedAddrsMap := make(map[string]bool) for _, addr := range jailAllowedAddrs { - _, err := sdk.ValAddressFromBech32(addr) + va, err := sdk.ValAddressFromBech32(addr) if err != nil { - log.Fatal(err) + return fmt.Errorf("invalid validator address in allowlist %q: %w", addr, err) } - - allowedAddrsMap[addr] = true + // Use canonical representation to match addr.String() later + allowedAddrsMap[va.String()] = true }Also prefer returning an error instead of process exit; see next comment.
Also applies to: 214-216
65-69: Avoid panic/log.Fatal; propagate errors to callerThis function is now error-returning, but several paths still panic or call log.Fatal, which aborts the process and undermines graceful export handling.
Replace panics/log.Fatal with error returns:
- if err != nil { - log.Fatal(err) - } + if err != nil { + return fmt.Errorf("invalid validator address in allowlist %q: %w", addr, err) + }- if err != nil { - panic(err) - } + if err != nil { + return fmt.Errorf("invalid validator address %q in delegation: %w", delegation.ValidatorAddress, err) + }- if err != nil { - panic(err) - } + if err != nil { + return fmt.Errorf("invalid validator address %q in delegation: %w", del.ValidatorAddress, err) + }- if err := app.DistrKeeper.Hooks().BeforeDelegationCreated(ctx, delAddr, valAddr); err != nil { - // never called as BeforeDelegationCreated always returns nil - panic(fmt.Errorf("error while incrementing period: %w", err)) - } + if err := app.DistrKeeper.Hooks().BeforeDelegationCreated(ctx, delAddr, valAddr); err != nil { + return fmt.Errorf("before-delegation-created hook failed: %w", err) + }- if err := app.DistrKeeper.Hooks().AfterDelegationModified(ctx, delAddr, valAddr); err != nil { - // never called as AfterDelegationModified always returns nil - panic(fmt.Errorf("error while creating a new delegation period record: %w", err)) - } + if err := app.DistrKeeper.Hooks().AfterDelegationModified(ctx, delAddr, valAddr); err != nil { + return fmt.Errorf("after-delegation-modified hook failed: %w", err) + }- if err != nil { - log.Fatal(err) - } + if err != nil { + return fmt.Errorf("apply validator set updates: %w", err) + }Also applies to: 92-93, 104-105, 111-117, 231-233
precompiles/async/async.go (1)
50-64: Fail fast if bankKeeper is nil to avoid runtime panics in balance hooksNewPrecompile now depends on a working BankKeeper. Add a guard so we don’t wire a nil handler and later crash at Before/AfterBalanceChange.
Apply this diff:
func NewPrecompile( asynckeeper asyncmodulekeeper.Keeper, bankKeeper evmcmn.BankKeeper, er *common.EthEventsRegistry, ) (*Precompile, error) { abi, err := LoadABI() if err != nil { return nil, err } + if bankKeeper == nil { + return nil, fmt.Errorf("async precompile: bankKeeper must not be nil") + } + p := Precompile{ Precompile: evmcmn.Precompile{ ABI: abi, KvGasConfig: storetypes.KVGasConfig(), TransientKVGasConfig: storetypes.TransientGasConfig(), }, asyncmodulekeeper: asynckeeper, eventsRegistry: er, queryServer: asyncmodulekeeper.NewQueryServerImpl(asynckeeper), }precompiles/sched/sched.go (2)
143-148: Bug: IsTransaction() always returns false; ExecuteCallbacksMethod must be treated as a transactionThis breaks gas/tx semantics and can misclassify state-changing calls as queries.
Apply this diff:
func (p *Precompile) IsTransaction(method *abi.Method) bool { - switch method.Name { - default: - return false - } + switch method.Name { + // transactions + case ExecuteCallbacksMethod: + return true + // queries + case CallbackByIdMethod, CallbacksMethod, GetAddressMethod: + return false + default: + return false + } }
52-56: Validate bankKeeper is provided before wiring the balance handlerMirror the async/act/warden constructors and fail fast on nil to avoid later crashes.
Apply this diff:
abi, err := LoadABI() if err != nil { return nil, err } + if bankKeeper == nil { + return nil, fmt.Errorf("sched precompile: bankKeeper must not be nil") + }Add the missing import:
// at the top import block import ( "embed" + "fmt" ... )warden/app/precompiles.go (2)
82-104: Validate codec parameter is non-nil.The function should validate that the codec parameter is not nil before using it.
func NewAvailableStaticPrecompiles( stakingKeeper stakingkeeper.Keeper, distributionKeeper distributionkeeper.Keeper, bankKeeper cmn.BankKeeper, erc20Keeper erc20Keeper.Keeper, transferKeeper transferkeeper.Keeper, channelKeeper *channelkeeper.Keeper, evmKeeper *evmkeeper.Keeper, govKeeper govkeeper.Keeper, slashingKeeper slashingkeeper.Keeper, wardenKeeper wardenkeeper.Keeper, actKeeper actkeeper.Keeper, oracleKeeper *oraclekeeper.Keeper, asyncKeeper asynckeeper.Keeper, schedKeeper schedkeeper.Keeper, codec codec.Codec, opts ...Option, ) map[common.Address]vm.PrecompiledContract { + if codec == nil { + panic("codec cannot be nil") + } + options := defaultOptionals() for _, opt := range opts { opt(&options) }
188-209: Add nil check for bankKeeper parameter.The bankKeeper is a critical dependency that should be validated.
wardenprecompiles, err := wardenprecompiles.NewWardenPrecompiles( bankKeeper, wardenKeeper, actKeeper, *oracleKeeper, asyncKeeper, schedKeeper, evmKeeper, ) if err != nil { - panic(err) + panic(fmt.Errorf("failed to create warden precompiles: %w", err)) }precompiles/precompiles.go (1)
36-44: Validate bankKeeper parameter.The bankKeeper is now a critical dependency for all precompiles and should be validated.
func NewWardenPrecompiles( bankKeeper evmcmn.BankKeeper, wardenkeeper wardenkeeper.Keeper, actkeeper actkeeper.Keeper, oraclekeeper oraclekeeper.Keeper, asynckeeper asynckeeper.Keeper, schedkeeper schedkeeper.Keeper, evmKeeper *evmkeeper.Keeper, ) (map[ethcmn.Address]vm.PrecompiledContract, error) { + if bankKeeper == nil { + return nil, fmt.Errorf("bankKeeper cannot be nil") + } + precompiles := make(map[ethcmn.Address]vm.PrecompiledContract) eventsRegistry := cmn.NewEthEventsRegistry()
🧹 Nitpick comments (58)
warden/x/act/module/simulation.go (1)
52-54: Empty slice return for ProposalMsgs — LGTM; align ProposalContents for consistencyReturning an empty slice here is good practice (stable JSON marshalling and fewer nil checks). For consistency across the module, consider also returning an empty slice from ProposalContents instead of nil.
Suggested change outside the selected lines:
func (AppModule) ProposalContents(_ module.SimulationState) []simtypes.WeightedProposalMsg { - return nil + return []simtypes.WeightedProposalMsg{} }warden/x/async/module/simulation.go (1)
65-67: Good switch to empty slice; mirror the pattern in ProposalContentsThe explicit empty slice avoids nil-vs-empty edge cases downstream. To keep the API style uniform in this file, mirror this in ProposalContents.
Suggested change outside the selected lines:
func (AppModule) ProposalContents(_ module.SimulationState) []simtypes.WeightedProposalMsg { - return nil + return []simtypes.WeightedProposalMsg{} }warden/x/sched/module/simulation.go (1)
46-49: Empty slice for ProposalMsgs — LGTM; minor cleanup in WeightedOperationsChange looks good. As a small cleanup, WeightedOperations currently allocates an empty slice via make(...) and then returns it. You can return the empty literal directly to avoid the intermediate variable.
Suggested change outside the selected lines:
func (am AppModule) WeightedOperations(simState module.SimulationState) []simtypes.WeightedOperation { - operations := make([]simtypes.WeightedOperation, 0) - - return operations + return []simtypes.WeightedOperation{} }warden/x/warden/module/simulation.go (1)
51-54: LGTM on empty slice; make ProposalContents consistentReturning []simtypes.WeightedProposalMsg{} is a sensible default. Consider switching ProposalContents to return an empty slice too for consistency with this method and with WeightedOperations’ empty slice style.
Suggested change outside the selected lines:
func (AppModule) ProposalContents(_ module.SimulationState) []simtypes.WeightedProposalMsg { - return nil + return []simtypes.WeightedProposalMsg{} }tests/framework/exec/warden_eth.go (2)
69-71: Avoid hard-coding GasLimit; let the backend estimate per txDropping the limit from 10,000,000,000 to 10,000,000 is sane, but keeping a fixed limit is still brittle across networks and tx types. Prefer estimation so calls don’t silently pass with too much headroom or fail on networks with tighter per-block limits.
- Let bind/backend estimate gas by setting GasLimit to 0.
- Keep legacy GasPrice only if the target EVM doesn’t support 1559; otherwise prefer GasFeeCap/GasTipCap. Verify what cosmos/evm exposes in your target version.
Apply this minimal change:
- auth.GasLimit = uint64(10_000_000) // in units + auth.GasLimit = 0 // let the backend estimate per call
88-92: 5s mining timeout is likely too aggressive; increase or derive from test deadline to avoid flakesUnder CI load or slower block times, 5 seconds can cause intermittent failures. Bump to a safer default (e.g., 30s) and include the tx hash in the failure message for faster triage.
Minimal, low-risk tweak:
- ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + ctx, cancel := context.WithTimeout(t.Context(), 30*time.Second) defer cancel() receipt, err := bind.WaitMined(ctx, c.Client, tx) - require.NoError(t, err) + require.NoError(t, err, "tx %s", tx.Hash())If you want a more robust approach, derive the timeout from t.Deadline() with a sane fallback and a small headroom; I can provide a helper for that on request.
warden/x/async/types/v1beta1/keys.go (2)
16-22: Consider adding RouterKey and MemStoreKey for completeness and future-proofing.These are commonly used alongside StoreKey in module wiring (routing msgs and mounting in-memory stores). Adding them now prevents ad-hoc literals later.
Apply within this const block:
const ( // ModuleName defines the module name. ModuleName = "async" // StoreKey defines the primary module store key. StoreKey = ModuleName + + // RouterKey defines the message routing key. + RouterKey = ModuleName + // MemStoreKey defines the in-memory store key. + MemStoreKey = "mem_" + ModuleName )
24-24: Minor consistency improvement: derive ParamsKey from ModuleName.Avoids accidental drift if ModuleName changes and keeps keys colocated.
Example:
var ParamsKey = []byte("p_" + ModuleName)warden/x/act/types/v1beta1/genesis_test.go (2)
29-33: Optional: run subtests in parallelIf
Validate()is pure and does not touch global state, addt.Parallel()inside the subtest to speed up the suite.- t.Run(tc.desc, func(t *testing.T) { - err := tc.genState.Validate() + t.Run(tc.desc, func(t *testing.T) { + t.Parallel() + err := tc.genState.Validate()
24-26: Clarify intent of zero-value genesis validity in TestGenesisState_ValidateThe test in
warden/x/act/types/v1beta1/genesis_test.gocurrently asserts that both the default and an empty (&GenesisState{}) genesis pass validation. That’s correct given thatValidate()simply returnsnil, but the intent could be clearer. Consider the following optional refinements:
- In the second test case, rename the description to explicitly call out a zero-value state:
• File:warden/x/act/types/v1beta1/genesis_test.go
• Changeto{ desc: "valid genesis state", genState: &types.GenesisState{}, valid: true, },{ desc: "zero-value genesis is valid", genState: &types.GenesisState{}, valid: true, },- If it was unintentional to treat a completely empty
GenesisStateas valid, adjustvalidtofalse(or remove this case) so that onlyDefaultGenesis()is accepted.- Although
Params.Validate()currently always returnsnil, review whether you intend to enforce non-zero defaults in future (e.g., requireMaxPendingTime > 0) rather than silently accepting an emptyParamsstruct.Diff suggestion:
--- a/warden/x/act/types/v1beta1/genesis_test.go +++ b/warden/x/act/types/v1beta1/genesis_test.go @@ -24,7 +24,7 @@ func TestGenesisState_Validate(t *testing.T) { { desc: "default is valid", genState: types.DefaultGenesis(), - valid: true, + valid: true, }, { - desc: "valid genesis state", + desc: "zero-value genesis is valid", genState: &types.GenesisState{}, valid: true, },warden/x/sched/types/v1beta1/genesis_test.go (1)
29-33: Optional: parallelize subtestsSame note as above; add
t.Parallel()within the subtest if safe.- t.Run(tc.desc, func(t *testing.T) { - err := tc.genState.Validate() + t.Run(tc.desc, func(t *testing.T) { + t.Parallel() + err := tc.genState.Validate()warden/x/warden/types/v1beta3/genesis_test.go (2)
29-33: Optional: subtest parallelizationConsider
t.Parallel()at the start of the subtest body if no shared mutable state is involved.- t.Run(tc.desc, func(t *testing.T) { - err := tc.genState.Validate() + t.Run(tc.desc, func(t *testing.T) { + t.Parallel() + err := tc.genState.Validate()
24-26: Confirm zero-value genesis is intentionally validThe
Validate()method only invokesParams.Validate()(which is currently a no-op), so both the default genesis and a zero-valueGenesisStatereturn no error. The existing tests already cover these two scenarios. For clarity, you may optionally rename the second test’s description:• In warden/x/warden/types/v1beta3/genesis_test.go, update the second case:
{ - desc: "valid genesis state", + desc: "zero-value genesis is valid", genState: &types.GenesisState{}, valid: true, },No functional changes are required.
tests/framework/snapshot/snapshots.go (1)
91-95: Make no_base_fee configurable via BuildOptions (default stays true).Hard-coding no_base_fee=true is fine for most tests, but we’ll likely want scenarios that exercise base-fee behavior. Expose it via an option so callers can flip it without a custom EditGenesis. This keeps the current default behavior while improving flexibility.
Apply within this hunk:
- genesis.Set("app_state.feemarket.params.no_base_fee", true) + genesis.Set("app_state.feemarket.params.no_base_fee", opts.NoBaseFee)And add this field to BuildOptions (outside this hunk):
type BuildOptions struct { Name string ChainID string Denom string KeyringBackend string Accounts []AccountOptions Spaces []SpaceOptions Keychains []KeychainOptions Plugins []string Precompiles []string EditSnapshot func(Builder) EditGenesis func(*GenesisBuilder) + // NoBaseFee controls app_state.feemarket.params.no_base_fee (default true to preserve existing behavior). + // Callers can set it explicitly when needed. + NoBaseFee bool }If you prefer to preserve the current default without touching all call sites, introduce a small defaulting shim:
// before genesis.Set(...) noBaseFee := opts.NoBaseFee if !optsProvided { // e.g., if you add a pointer field instead: *bool noBaseFee = true } genesis.Set("app_state.feemarket.params.no_base_fee", noBaseFee)warden/x/sched/keeper/abci.go (1)
19-22: Alias geth crypto import to avoid ambiguity with stdlib crypto packages.Minor readability nit: importing github.com/ethereum/go-ethereum/crypto as crypto alongside crypto/ecdsa can confuse readers and IDEs. Prefer an explicit alias.
- "github.com/ethereum/go-ethereum/crypto" + gethcrypto "github.com/ethereum/go-ethereum/crypto"…and update usages:
- tx.From = crypto.PubkeyToAddress(privkey.PublicKey).Bytes() + tx.From = gethcrypto.PubkeyToAddress(privkey.PublicKey).Bytes()warden/app/upgrades.go (1)
3-5: Use pointer receiver for RegisterUpgradeHandlers to avoid copying App.This method will eventually mutate registries/keepers; using a value receiver would mutate a copy. Switch to a pointer receiver preemptively to prevent subtle bugs when you add handlers.
-func (app App) RegisterUpgradeHandlers() { +func (app *App) RegisterUpgradeHandlers() { // No upgrades registered yet }warden/app/vemanager/vemanager.go (1)
44-50: Log panic value and stack with the SDK logger before re-panicking.Printing to stderr works but is easy to miss and lacks chain context. Log via ctx.Logger() with the panic value and stack; then re-panic to preserve consensus behavior. This aids debugging in production setups.
- defer func() { - if err := recover(); err != nil { - debug.PrintStack() - panic(err) - } - }() + defer func() { + if r := recover(); r != nil { + ctx.Logger().Error("panic in VoteExtensionManager.ExtendVoteHandler", + "panic", r, + "stack", string(debug.Stack()), + "height", ctx.BlockHeight(), + ) + panic(r) + } + }()Optional follow-up: add the same guard in VerifyVoteExtensionHandler for symmetry.
cmd/wardend/config/chain_id.go (1)
3-9: Wrap errors with context and validate the returned chain ID.Improve diagnosability when the file is missing/malformed and fail fast on empty chain IDs.
package config import ( + "fmt" "path/filepath" "github.com/cosmos/cosmos-sdk/client/config" "github.com/spf13/viper" ) // GetChainIDFromHome returns the chain ID from the client configuration // in the given home directory. func GetChainIDFromHome(home string) (string, error) { v := viper.New() v.AddConfigPath(filepath.Join(home, "config")) v.SetConfigName("client") v.SetConfigType("toml") - if err := v.ReadInConfig(); err != nil { - return "", err + if err := v.ReadInConfig(); err != nil { + return "", fmt.Errorf("reading client config from %s: %w", filepath.Join(home, "config", "client.toml"), err) } conf := new(config.ClientConfig) - if err := v.Unmarshal(conf); err != nil { - return "", err + if err := v.Unmarshal(conf); err != nil { + return "", fmt.Errorf("unmarshalling client config: %w", err) } - return conf.ChainID, nil + if conf.ChainID == "" { + return "", fmt.Errorf("chain ID is empty in %s", filepath.Join(home, "config", "client.toml")) + } + return conf.ChainID, nil }Also applies to: 18-29
prophet/registry.go (4)
41-46: Return plugin names in deterministic orderIteration order over maps is randomized; returning unsorted names can produce flakey tests and nondeterministic UX. Sort before returning.
-import ( - "fmt" - "sync" -) +import ( + "fmt" + "sort" + "sync" +) @@ - plugins := make([]string, 0, len(registry.plugins)) - for name := range registry.plugins { + plugins := make([]string, 0, len(registry.plugins)) + for name := range registry.plugins { plugins = append(plugins, name) } - return plugins + sort.Strings(plugins) + return plugins
23-25: Use a plain formatted string in panic to avoid wrapping an errorPanicking with an error is fine, but using a string reduces cognitive overhead and avoids an extra allocation for the error value.
- if _, found := registry.plugins[name]; found { - panic(fmt.Errorf("plugin %s registered twice", name)) - } + if _, found := registry.plugins[name]; found { + panic(fmt.Sprintf("plugin %s registered twice", name)) + }
8-8: Fix stale comment: “tasks plugins” → “plugins”Aligns the comment with the rename from tasks to plugins.
-// registry is a global registry of tasks plugins. +// registry is a global registry of plugins.
30-35: Consider surfacing “not found” explicitlygetPlugin returns nil when missing, which can hide configuration errors until a later method call panics. If you can change call sites, prefer returning (Plugin, bool).
-func getPlugin(name string) Plugin { +func getPlugin(name string) (Plugin, bool) { registry.rw.RLock() defer registry.rw.RUnlock() - return registry.plugins[name] + p, ok := registry.plugins[name] + return p, ok }warden/app/oracle.go (2)
259-265: Improve recovered panic logging in PreBlockerdebug.PrintStack() prints to stderr and re-panicking may lose the original stack in some setups. Log the stack via ctx.Logger() and rethrow the original value for better observability.
return func(ctx sdk.Context, req *cometabci.RequestFinalizeBlock) (*sdk.ResponsePreBlock, error) { - defer func() { - if err := recover(); err != nil { - debug.PrintStack() - panic(err) - } - }() + defer func() { + if r := recover(); r != nil { + ctx.Logger().Error("preblock panic", "err", r, "stack", string(debug.Stack())) + panic(r) + } + }()
317-319: Clarify the purpose of TxSelector or remove until usedTxSelector embeds App but has no methods or interface conformance in this file. If it’s a placeholder for mempool selection, add a doc comment and its intended interface, or drop until it’s actually used.
Would you like me to wire this to the EVM mempool interfaces (e.g., provide selection/filter funcs) in a follow-up?
warden/app/ante/options.go (2)
13-23: Doc string says IBC/Wasm keepers are required; Validate only enforces wasm config/store serviceEither adjust the doc or enforce presence of the keepers that are marked as “required”.
-// HandlerOptions extend the SDK's AnteHandler options by requiring the IBC -// -// channel keeper, wasm keeper and evm keepers. +// HandlerOptions extends the SDK/EVM AnteHandler options and includes IBC and +// CosmWasm-specific knobs needed by Wardend’s ante chain.
25-40: Enforce non-nil WasmKeeper/IBCKeeper if they are truly requiredIf your ante chain depends on these keepers, fail fast here.
func (options HandlerOptions) Validate() error { if err := options.HandlerOptions.Validate(); err != nil { return err } if options.NodeConfig == nil { return errors.New("wasm config is required for ante builder") } if options.TXCounterStoreService == nil { return errors.New("wasm store service is required for ante builder") } + if options.WasmKeeper == nil { + return errors.New("wasm keeper is required for ante builder") + } + + // If IBC ante steps are wired, enforce IBCKeeper as well. + // Remove this check if truly optional in your wiring. + if options.IBCKeeper == nil { + return errors.New("ibc keeper is required for ante builder") + } + return nil }If IBCKeeper/WasmKeeper are optional in your current ante path, ignore this and align the comment instead (previous note).
justfile (1)
28-30: Harden jq extraction and drop quotes from variables
- Use -r for raw output to avoid quoted strings.
- Handle both “_contract_address” and “contract_address” keys for broader wasm versions.
- CODE_ID=$(wardend tx wasm store {{contract}} --from {{from}} -y --gas 10000000 --fees 1award | wardend q wait-tx -o json | jq '.events[] | select(.type == "store_code") | .attributes[] | select(.key == "code_id") | .value | tonumber') - ADDR=$(wardend tx wasm instantiate $CODE_ID '{}' --from {{from}} --label "{{if label == "" { "default" } else { label } }}" --no-admin --fees 1award -y | wardend q wait-tx -o json | jq '.events[] | select(.type == "instantiate") | .attributes[] | select(.key == "_contract_address") | .value') + CODE_ID=$(wardend tx wasm store {{contract}} --from {{from}} -y --gas 10000000 --fees 1award \ + | wardend q wait-tx -o json \ + | jq -r '.events[] | select(.type == "store_code") | .attributes[] | select(.key == "code_id") | .value' ) + ADDR=$(wardend tx wasm instantiate "$CODE_ID" '{}' --from {{from}} --label "{{if label == "" { "default" } else { label } }}" --no-admin --fees 1award -y \ + | wardend q wait-tx -o json \ + | jq -r '.events[] | select(.type == "instantiate") | .attributes[] | select(.key == "_contract_address" or .key == "contract_address") | .value' )go.mod (2)
55-57: Track the evm/evmd RC and pseudo-version pinningYou’re on cosmos/evm v0.5.0-rc.0 and a pseudo-version for evmd (likely a commit from the pending PR). Once cosmos/evm#416 lands, please move to a tagged release for both to reduce supply-chain churn.
Would you like a follow-up patch that bumps these to the first stable release once the PR merges?
11-25: Consolidate Pebble Version Pinning in go.modI’ve confirmed that your
go.modboth:
- Pins Pebble to the older commit
v0.0.0-20230928194634-aa077af62593via areplaceat line 12, and- Still lists an indirect
requirefor Pebblev1.1.5at line 187.While the
replaceoverride makes this safe, having both can be confusing for future maintainers. To tidy this up, please consider one of the following:
- Run
go mod tidyto remove the now-redundant indirectrequire github.com/cockroachdb/pebble v1.1.5.- Or, if you intentionally want to keep the indirect entry as documentation, add a brief comment above it explaining that the
replaceis required to satisfy Geth’s build constraints.Optional: update the in-file comment on the
replaceto reference why exactly this commit is chosen (e.g., “pin to this commit to avoid breaking changes in Geth’s build”).warden/app/export.go (2)
75-81: Ignored errors when withdrawing rewards/commissionWithdrawValidatorCommission and WithdrawDelegationRewards errors are ignored. If distribution state is inconsistent, you’ll silently proceed with an invalid export.
Handle and aggregate errors:
-_, _ = app.DistrKeeper.WithdrawValidatorCommission(ctx, sdk.ValAddress(val.GetOperator())) +if _, err := app.DistrKeeper.WithdrawValidatorCommission(ctx, sdk.ValAddress(val.GetOperator())); err != nil { + return fmt.Errorf("withdraw validator commission %s: %w", val.GetOperator(), err) +}-_, _ = app.DistrKeeper.WithdrawDelegationRewards(ctx, delAddr, valAddr) +if _, err := app.DistrKeeper.WithdrawDelegationRewards(ctx, delAddr, valAddr); err != nil { + return fmt.Errorf("withdraw delegation rewards %s->%s: %w", delAddr, valAddr, err) +}Also applies to: 97-98
133-153: Reduce err shadowing inside closure to improve clarityMultiple short declarations with err (scraps, feePool, Set, Hooks) shadow outer err, which can be confusing. It’s correct but brittle.
Refactor to use distinct names or a single scoped err to avoid shadowing.
precompiles/json/json.go (1)
96-104: Optional: guard against unknown method names in RunCurrently, an unknown method name yields bz=nil, err=nil if not matched. Consider a default branch returning a clear error.
switch method.Name { // queries case BuildMethod: bz, err = p.Build(ctx, method, args) case ParseMethod: bz, err = p.Parse(ctx, method, args) + default: + return nil, fmt.Errorf("json precompile: unsupported method %s", method.Name) }precompiles/act/act.go (2)
96-149: Defensive nil-guard around balance handler callsIf future refactors forget to set the handler, these calls will panic.
- p.GetBalanceHandler().BeforeBalanceChange(ctx) + if h := p.GetBalanceHandler(); h != nil { + h.BeforeBalanceChange(ctx) + } ... - if err = p.GetBalanceHandler().AfterBalanceChange(ctx, stateDB); err != nil { + if h := p.GetBalanceHandler(); h != nil { + if err = h.AfterBalanceChange(ctx, stateDB); err != nil { + return nil, err + } + }
119-121: Intentional difference between evm.Origin and contract.Caller()?VoteForActionMethod uses contract.Caller() while other tx paths use evm.Origin. If this is intentional (e.g., voting by caller vs origin), add a short comment to prevent regressions.
- bz, err = p.VoteForActionMethod(ctx, evm.Origin, contract.Caller(), stateDB, method, args) + // Use contract.Caller() to attribute votes to the immediate caller (not necessarily tx origin). + bz, err = p.VoteForActionMethod(ctx, evm.Origin, contract.Caller(), stateDB, method, args)cmd/wardend/config/config.go (2)
41-45: SetFullFundraiserPath: LGTM; keep nolint but reference rationaleUsing types.BIP44HDPath with nolint is deliberate. Add a brief comment linking to the SDK deprecation note to reduce churn for future maintainers.
- config.SetFullFundraiserPath(types.BIP44HDPath) //nolint:staticcheck + //nolint:staticcheck -- SetFullFundraiserPath is deprecated in SDK, maintained for hd path compatibility. + config.SetFullFundraiserPath(types.BIP44HDPath)
29-31: Make EVMChainID Configurable Across All CommandsThe hard-coded constant
// cmd/wardend/config/config.go:30 EVMChainID = uint64(1337)is currently pulled in by multiple entry points and configuration constructors:
- cmd/wardend/cmd/testnet.go
• Lines ~250–254:• Lines ~646–658: passed into the simulated‐app builder viaevm := cosmosevmserverconfig.DefaultEVMConfig() evm.EVMChainID = evmdconfig.EVMChainIDevmdconfig.EVMChainID- cmd/wardend/cmd/root.go
• Line 68: passed tosimtestutil.NewAppaswardendconfig.EVMChainID
• Lines 139–142: overridden when a--chain-idflag is provided and parsed byparseEVMChainID- cmd/wardend/config/wardend_config.go
• Line 141–142:evmCfg := cosmosevmserverconfig.DefaultEVMConfig() evmCfg.EVMChainID = evmChainIDTo allow non-dev networks to override the chain ID safely:
- Expose an
evmChainIDfield in your application’s config struct (backed by Viper/TOML, environment or CLI flag).- Default that field to
1337when unspecified.- In all call sites above, drop direct references to the constant and instead read from the new config field.
- Hook up a CLI flag (e.g.
--evm-chain-id) toparseEVMChainIDso it flows into the shared config used by both therootandtestnetcommands, as well as thewardend_configbuilder.This preserves the local/dev default while enabling easy overrides for testnet and mainnet.
warden/app/ante/evm_handler.go (1)
9-22: Guard TxListener decorator behind a nil checkSome environments won’t provide a PendingTxListener; adding it unconditionally risks a nil deref inside the decorator.
Apply this diff:
func newMonoEVMAnteHandler(options HandlerOptions) sdk.AnteHandler { - decorators := []sdk.AnteDecorator{ - evmante.NewEVMMonoDecorator( - options.AccountKeeper, - options.FeeMarketKeeper, - options.EvmKeeper, - options.MaxTxGasWanted, - ), - ante.NewTxListenerDecorator(options.PendingTxListener), - } + decorators := []sdk.AnteDecorator{ + evmante.NewEVMMonoDecorator( + options.AccountKeeper, + options.FeeMarketKeeper, + options.EvmKeeper, + options.MaxTxGasWanted, + ), + } + if options.PendingTxListener != nil { + decorators = append(decorators, ante.NewTxListenerDecorator(options.PendingTxListener)) + } return sdk.ChainAnteDecorators(decorators...) }localnet.just (1)
76-76: Use readiness loop instead of fixed sleep for contract deploymentA fixed 5s delay is flaky on slower machines/CI. Poll the RPC to ensure the node is up before deploying.
Apply this diff:
- sleep 5 + # Wait up to 60s for RPC to become available + for i in {1..60}; do + if command -v curl >/dev/null && curl -sf http://127.0.0.1:26657/status >/dev/null; then + break + fi + sleep 1 + doneprecompiles/warden/warden.go (4)
53-76: Guard against a nil BankKeeper in NewPrecompile.If a nil bankKeeper bubbles in, balance handling will panic later at runtime. Fail fast in the constructor.
Apply this diff:
func NewPrecompile( wardenkeeper wardenmodulekeeper.Keeper, actkeeper actmodulekeeper.Keeper, - bankKeeper evmcmn.BankKeeper, + bankKeeper evmcmn.BankKeeper, e *precommon.EthEventsRegistry, ) (*Precompile, error) { + if bankKeeper == nil { + return nil, fmt.Errorf("warden precompile: bankKeeper must not be nil") + } @@ - p.SetAddress(common.HexToAddress(PrecompileAddress)) + p.SetAddress(p.Address()) p.SetBalanceHandler(bankKeeper)
200-235: Avoid panicking on unknown ABI method in IsTransaction.Even if “should never happen”, a panic in a precompile can take down the VM. Return false instead; Run will still fail gracefully when method dispatch doesn’t match.
Apply this diff:
default: - panic(fmt.Errorf("warden precompile: method not exists: %s", method)) + return false }
85-103: RequiredGas override is fine; keep an eye on ABI desync.The override and call into embedded Precompile.RequiredGas look right. If ABI changes but IsTransaction isn’t updated, gas classification may drift. Consider a small unit test that iterates ABI methods and asserts IsTransaction covers all names.
111-116: Standardize conditional AfterBalanceChange across all precompilesVerified that each of the four precompiles invokes
p.GetBalanceHandler().BeforeBalanceChange(ctx) … if err = p.GetBalanceHandler().AfterBalanceChange(ctx, stateDB); err != nil { return nil, err }in symmetric positions. To guarantee that
AfterBalanceChangestill runs on every successful path (including early returns) and always afterHandleGasError(LIFO defer order), we should replace the explicit call with a conditional defer in each precompile.Affected locations:
- precompiles/warden/warden.go (Before: L111–116; After: L188–190)
- precompiles/sched/sched.go (Before: L106; After: L136)
- precompiles/act/act.go (Before: L103; After: L144)
- precompiles/async/async.go (Before: L103; After: L134)
Example diff for warden.go:
p.GetBalanceHandler().BeforeBalanceChange(ctx) // This handles any out of gas errors that may occur during the execution of a precompile tx or query. // It avoids panics and returns the out of gas error so the EVM can continue gracefully. + // Defer balance finalization; will only run if err stays nil and will execute after HandleGasError + // since this defer is registered before it (LIFO). + defer func() { + if err == nil { + _ = p.GetBalanceHandler().AfterBalanceChange(ctx, stateDB) + } + }() defer evmcmn.HandleGasError(ctx, contract, initialGas, &err)() @@ - if err = p.GetBalanceHandler().AfterBalanceChange(ctx, stateDB); err != nil { - return nil, err - }Please apply this pattern to the other three precompiles to maintain consistency.
cmd/wardend/main.go (1)
24-28: Minor: make setupSDKConfig idempotent-safe across multiple invocations.SDK config is a singleton; SetBech32Prefixes is benign but you could guard with a sync.Once if setupSDKConfig might be reused in tests.
warden/app/config.go (2)
19-22: Prefer sync.Once over a package-scoped boolean for thread-safety and testability.While current init is single-threaded, sync.Once clarifies intent and avoids races in concurrent test runners. Expose a test-only reset if needed.
Example refactor (outside the selected lines, illustrative):
// at top of file var setupEVMOnce sync.Once func (app *App) setupEVM(evmChainID uint64) error { var setupErr error setupEVMOnce.Do(func() { eip712.SetEncodingConfig(app.legacyAmino, app.interfaceRegistry, evmChainID) if err := setBaseDenom(coinInfo); err != nil { setupErr = err return } ethCfg := evmtypes.DefaultChainConfig(evmChainID) setupErr = evmtypes.NewEVMConfigurator(). WithChainConfig(ethCfg). WithEVMCoinInfo(coinInfo). Configure() }) return setupErr }
24-37: Validate legacyAmino/interfaceRegistry non-nil before SetEncodingConfig.A defensive check can surface mis-ordered app wiring early with a clear error.
Proposed guard (within the function body):
if app.legacyAmino == nil || app.interfaceRegistry == nil { return fmt.Errorf("setupEVM: codec/registry not initialized") }warden/app/ante/ante.go (3)
13-17: Typo: “it's respective handler” → “its respective handler”.Apply this diff:
-// transaction-level processing (e.g. fee payment, signature verification) before -// being passed onto it's respective handler. +// transaction-level processing (e.g. fee payment, signature verification) before +// being passed onto its respective handler.
17-22: Add early validation ofHandlerOptionsinNewAnteHandlerTo ensure required keepers are wired and to fail fast, invoke the existing
Validate()method onHandlerOptionsat the start of the ante handler closure.• File:
warden/app/ante/ante.go
• Location: insideNewAnteHandler, immediately before any further logicSuggested diff:
func NewAnteHandler(options HandlerOptions) sdk.AnteHandler { return func( ctx sdk.Context, tx sdk.Tx, sim bool, ) (newCtx sdk.Context, err error) { + // Fail fast on invalid/missing options + if err := options.Validate(); err != nil { + return ctx, err + } var anteHandler sdk.AnteHandler // … rest of handler setup …The
Validate()method is already implemented onHandlerOptions(seewarden/app/ante/options.go:26), so this simple invocation provides early error detection.
31-37: Useproto.MessageNamefor ExtensionOptions to avoid hard-coded URLsTo make the code resilient to upstream changes, derive the full type URL from the proto message types instead of embedding string literals. Based on our search, the two extension options are defined in:
wardenjs/proto/ethermint/evm/v1/tx.proto
• messageExtensionOptionsEthereumTxwardenjs/proto/ethermint/types/v1/dynamic_fee.proto
• messageExtensionOptionDynamicFeeTxWe can replace the hard-coded cases with:
-import ( - // … other imports -) +import ( + "google.golang.org/protobuf/proto" + vmv1 "your/module/path/wardenjs/proto/ethermint/evm/v1" + typesv1 "your/module/path/wardenjs/proto/ethermint/types/v1" +) // … switch extOpt.TypeUrl { - case "/cosmos.evm.vm.v1.ExtensionOptionsEthereumTx": + case "/" + proto.MessageName(&vmv1.ExtensionOptionsEthereumTx{}): // handle as *evmtypes.MsgEthereumTx anteHandler = newMonoEVMAnteHandler(options) - case "/cosmos.evm.types.v1.ExtensionOptionDynamicFeeTx": + case "/" + proto.MessageName(&typesv1.ExtensionOptionDynamicFeeTx{}): // cosmos-sdk tx with dynamic fee extension anteHandler = newCosmosAnteHandler(options) default: // … }Points to verify:
- Confirm the Go module path and package aliases (
vmv1,typesv1) match thego_packageoption in each.proto.- Update imports in
warden/app/ante/ante.goto reference the correct generated Go packages.- Run
go build ./warden/app/anteto ensure nothing breaks.warden/app/ante/sched_handler.go (2)
25-28: Comment clarity.Tighten the warning to avoid contractions in docs.
Apply this diff:
- // This transaction will bypass normal authentication checks such as signature and nonce. - // It's dangerous and should only be allowed if built by the x/sched module. + // This transaction bypasses normal authentication checks such as signature and nonce. + // It is dangerous and should only be allowed if built by the x/sched module.
39-51: Defense-in-depth: restrict messages to EVM messages only (already enforced) and consider empty-tx guard.You already unpack each msg via UnpackEthMsg which enforces EVM messages. Optionally add a guard to reject empty txs early for clearer errors.
Example:
if len(tx.GetMsgs()) == 0 { return ctx, errorsmod.Wrapf(errortypes.ErrInvalidRequest, "empty system transaction") }cmd/wardend/config/wardend_config.go (2)
43-61: Consider documenting the module account permissions.The
maccPermsvariable contains critical security permissions for module accounts. Consider adding documentation for each permission to explain why specific permissions are granted to each module.Add comments explaining the rationale for each module's permissions:
// module account permissions. var maccPerms = map[string][]string{ + // FeeCollector receives transaction fees authtypes.FeeCollectorName: nil, + // Distribution module needs no special permissions distrtypes.ModuleName: nil, + // IBC transfer needs minter/burner for cross-chain token transfers ibctransfertypes.ModuleName: {authtypes.Minter, authtypes.Burner}, // ... continue for other modules
120-186: Consider extracting magic values as constants.The configuration initialization contains several hardcoded values (timeout durations, ports, etc.). Consider extracting these as named constants for better maintainability.
+const ( + defaultOracleTimeout = time.Second * 2 + defaultOracleAddress = "localhost:8080" +) func InitAppConfig(denom string, evmChainID uint64) (string, any) { // ... oracleCfg := oracleconfig.AppConfig{ Enabled: true, - OracleAddress: "localhost:8080", - ClientTimeout: time.Second * 2, + OracleAddress: defaultOracleAddress, + ClientTimeout: defaultOracleTimeout, MetricsEnabled: true, }cmd/wardend/cmd/root.go (1)
161-164: Handle potential error from EnhanceRootCommand.While panicking might be acceptable during initialization, consider logging more context.
if err := autoCliOpts.EnhanceRootCommand(rootCmd); err != nil { - panic(err) + panic(fmt.Errorf("failed to enhance root command with AutoCLI: %w", err)) }warden/app/app.go (5)
915-918: Hard-coded BlockGasLimit for EVMMempool; should sync with consensus params
BlockGasLimit: 100_000_000is a static value and may diverge from the actual consensus block gas limit, leading to admission discrepancies.Consider one of:
- Read the current block gas limit from consensus params at BeginBlock and update the mempool configuration accordingly.
- Source the limit from a single config (e.g., app options or FeeMarket/EVM params) to keep it in sync with consensus.
1425-1431: Local variable name shadows the imported url package
url, err := url.Parse(u)shadows thenet/urlimport, reducing readability and increasing risk of mistakes in adjacent code.Rename the local variable:
- url, err := url.Parse(u) + ppURL, err := url.Parse(u) if err != nil { panic(fmt.Errorf("invalid pricepred url: %s", u)) } - prophet.Register("pricepred", pricepred.NewPricePredictorSolidity(url)) + prophet.Register("pricepred", pricepred.NewPricePredictorSolidity(ppURL))
1318-1337: Minor: unnecessary zero-length hint in map construction
make(map[string]appmodule.AppModule, 0)doesn’t need the zero capacity. Prefermake(map[string]appmodule.AppModule)for idiomatic Go.- modules := make(map[string]appmodule.AppModule, 0) + modules := make(map[string]appmodule.AppModule)
1282-1306: Close() should tolerate nil mempool without calling GetMempool() method chain
app.GetMempool()may be nil if EVM chain config was not set or mempool wasn’t initialized; your type assertion guards usage, which is good. Consider adding a fast path nil check to avoid the method call altogether (micro-optimization, optional).
1126-1131: GetMemKey returns from a nil map (memKeys is never initialized)
memKeysis declared on the struct but not initialized. Reads from a nil map return zero values, so this is safe, but confusing. Either initializememKeysor remove it if unused.
Pitasi
force-pushed
the
remove-depinject
branch
from
ecd3266 to
7ecd4da
Compare
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 (5)
warden/app/config.go (1)
10-17: Avoid silent no-op and detect EVM chain ID mismatches after first initializationsetupEVM returns nil on repeated calls. If a subsequent call passes a different evmChainID, the function silently does nothing, leaving a stale EIP-155 chain ID for EIP-712 and EVM config. This can lead to signing/verification inconsistencies and hard-to-debug behavior in multi-app test runs or tooling that constructs multiple App instances.
Introduce a sealedChainID guard and error on mismatch so misconfiguration is surfaced early.
Apply this diff:
var sealed = false +var sealedChainID uint64 func (app *App) setupEVM(evmChainID uint64) error { if sealed { - return nil + if evmChainID != sealedChainID { + return fmt.Errorf("setupEVM already initialized with chain ID %d (got %d)", sealedChainID, evmChainID) + } + return nil } @@ sealed = true + sealedChainID = evmChainIDAlso applies to: 19-22
warden/app/export.go (4)
65-71: Do not abort the process; return errors instead of log.FatalIn prepForZeroHeightGenesis, encountering an invalid validator address logs a fatal and exits the process. This defeats callers’ error handling and can kill nodes/tools unexpectedly.
Apply this diff:
for _, addr := range jailAllowedAddrs { _, err := sdk.ValAddressFromBech32(addr) if err != nil { - log.Fatal(err) + return fmt.Errorf("invalid validator address %q: %w", addr, err) } allowedAddrsMap[addr] = true }
90-98: Avoid panics when parsing addresses; propagate errorsThe current code panics on malformed bech32 addresses. Return errors instead so the caller can handle them.
Apply this diff:
- valAddr, err := sdk.ValAddressFromBech32(delegation.ValidatorAddress) - if err != nil { - panic(err) - } - delAddr := sdk.MustAccAddressFromBech32(delegation.DelegatorAddress) + valAddr, err := sdk.ValAddressFromBech32(delegation.ValidatorAddress) + if err != nil { + return fmt.Errorf("invalid validator address in delegation: %w", err) + } + delAddr, err := sdk.AccAddressFromBech32(delegation.DelegatorAddress) + if err != nil { + return fmt.Errorf("invalid delegator address in delegation: %w", err) + }And similarly for the re-initialization loop:
- valAddr, err := sdk.ValAddressFromBech32(del.ValidatorAddress) - if err != nil { - panic(err) - } - delAddr := sdk.MustAccAddressFromBech32(del.DelegatorAddress) + valAddr, err := sdk.ValAddressFromBech32(del.ValidatorAddress) + if err != nil { + return fmt.Errorf("invalid validator address in delegation: %w", err) + } + delAddr, err := sdk.AccAddressFromBech32(del.DelegatorAddress) + if err != nil { + return fmt.Errorf("invalid delegator address in delegation: %w", err) + }Also applies to: 102-108
109-117: Hooks: replace panics with error propagationBoth BeforeDelegationCreated and AfterDelegationModified error paths panic. Keep the function error-returning and propagate failures.
Apply this diff:
- if err := app.DistrKeeper.Hooks().BeforeDelegationCreated(ctx, delAddr, valAddr); err != nil { - // never called as BeforeDelegationCreated always returns nil - panic(fmt.Errorf("error while incrementing period: %w", err)) - } + if err := app.DistrKeeper.Hooks().BeforeDelegationCreated(ctx, delAddr, valAddr); err != nil { + return fmt.Errorf("before-delegation-created hook: %w", err) + } @@ - if err := app.DistrKeeper.Hooks().AfterDelegationModified(ctx, delAddr, valAddr); err != nil { - // never called as AfterDelegationModified always returns nil - panic(fmt.Errorf("error while creating a new delegation period record: %w", err)) - } + if err := app.DistrKeeper.Hooks().AfterDelegationModified(ctx, delAddr, valAddr); err != nil { + return fmt.Errorf("after-delegation-modified hook: %w", err) + }
230-233: Avoid log.Fatal on ApplyAndReturnValidatorSetUpdateslog.Fatal exits the process; propagate the error instead.
Apply this diff:
_, err = app.StakingKeeper.ApplyAndReturnValidatorSetUpdates(ctx) if err != nil { - log.Fatal(err) + return fmt.Errorf("apply validator set updates: %w", err) }
♻️ Duplicate comments (7)
warden/app/export.go (1)
225-228: Iterator close error is swallowed; return itThe error from iter.Close() is logged and the function returns nil, losing the error. Return the Close() error wrapped with context.
Apply this diff:
if err := iter.Close(); err != nil { app.Logger().Error("error while closing the key-value store reverse prefix iterator: ", err) - return nil + return fmt.Errorf("closing validator reverse iterator: %w", err) }cmd/wardend/cmd/root.go (3)
434-441: Fix temp directory prefix namePrefix should reflect the binary (“wardend”), not “wasmd”.
Apply this diff:
-var tempDir = func() string { - dir, err := os.MkdirTemp("", "wasmd") +var tempDir = func() string { + dir, err := os.MkdirTemp("", "wardend") if err != nil { panic("failed to create temp dir: " + err.Error()) } return dir }
134-146: Replace panics with returned errors in PreRunEPreRunE should never panic on flag parsing. Return errors so Cobra can render them.
Apply this diff:
chainID, err := cmd.Flags().GetString(flags.FlagChainID) if err != nil { - panic(err) + return fmt.Errorf("failed to get chain ID: %w", err) } evmChainID := wardendconfig.EVMChainID if chainID != "" { - var err error - evmChainID, err = parseEVMChainID(chainID) - if err != nil { - panic(err) - } + var pErr error + evmChainID, pErr = parseEVMChainID(chainID) + if pErr != nil { + return fmt.Errorf("failed to parse EVM chain ID from %q: %w", chainID, pErr) + } }
422-431: Validate chain ID format before slicing; clearer error messagesparseEVMChainID assumes the presence and order of '_' and '-'. Add validation to avoid out-of-range slicing and produce clearer errors.
Apply this diff:
func parseEVMChainID(chainID string) (uint64, error) { from := strings.LastIndexByte(chainID, '_') to := strings.LastIndexByte(chainID, '-') - - evmChainID, err := strconv.ParseUint(chainID[from+1:to], 10, 64) + if from == -1 || to == -1 || from >= to { + return 0, fmt.Errorf("invalid chain ID format (expected '<prefix>_<number>-<suffix>'): %s", chainID) + } + part := chainID[from+1 : to] + if part == "" { + return 0, fmt.Errorf("empty numeric section in chain ID: %s", chainID) + } + evmChainID, err := strconv.ParseUint(part, 10, 64) if err != nil { return 0, fmt.Errorf("can't parse evm chain id from %s: %w", chainID, err) } return evmChainID, nil }warden/app/app.go (3)
801-841: PreBlockers ordering is configured but never wired into BaseAppYou set SetOrderPreBlockers, but BaseApp never gets a PreBlocker. Without app.SetPreBlocker, pre-blockers won’t execute at all during FinalizeBlock.
Apply this wiring alongside Begin/End blocker registration:
- app.SetInitChainer(app.InitChainer) - app.SetBeginBlocker(app.BeginBlocker) - app.SetEndBlocker(app.EndBlocker) + app.SetInitChainer(app.InitChainer) + app.SetPreBlocker(app.PreBlocker) + app.SetBeginBlocker(app.BeginBlocker) + app.SetEndBlocker(app.EndBlocker)Also applies to: 973-977
912-927: EVM mempool is constructed before AnteHandler is set, leading to a nil/old ante in mempoolThe EVMMempool is created using AnteHandler: app.GetAnteHandler() before app.SetAnteHandler(...) runs. This captures a nil/incorrect handler and may bypass ante checks in mempool.
- Remove the premature initialization:
- // mempool initialization (EVM priority nonce mempool) - if evmtypes.GetChainConfig() != nil { - // TODO: Get the actual block gas limit from consensus parameters - mempoolConfig := &evmmempool.EVMMempoolConfig{ - AnteHandler: app.GetAnteHandler(), - BlockGasLimit: 100_000_000, - } - - evmMempool := evmmempool.NewExperimentalEVMMempool(app.CreateQueryContext, logger, app.EVMKeeper, app.FeeMarketKeeper, app.txConfig, app.clientCtx, mempoolConfig) - app.EVMMempool = evmMempool - - app.SetMempool(evmMempool) - checkTxHandler := evmmempool.NewCheckTxHandler(evmMempool) - app.SetCheckTxHandler(checkTxHandler) - }
- Re-create the mempool after ante/post handlers are configured (right after app.setPostHandler()):
@@ app.setPostHandler() + + // mempool initialization (EVM priority nonce mempool) + if evmtypes.GetChainConfig() != nil { + // TODO: derive the block gas limit from consensus parameters + mempoolConfig := &evmmempool.EVMMempoolConfig{ + AnteHandler: app.GetAnteHandler(), // now set by setAnteHandler + BlockGasLimit: 100_000_000, + } + evmMempool := evmmempool.NewExperimentalEVMMempool( + app.CreateQueryContext, logger, + app.EVMKeeper, app.FeeMarketKeeper, + app.txConfig, app.clientCtx, + mempoolConfig, + ) + app.EVMMempool = evmMempool + app.SetMempool(evmMempool) + app.SetCheckTxHandler(evmmempool.NewCheckTxHandler(evmMempool)) + }Follow-ups:
- If EVMMempool caches the ante handler internally by value, prefer re-instantiating on handler changes or exposing a setter.
- Ensure app.clientCtx is initialized before mempool construction or pass a minimal default client.Context.
Also applies to: 973-1006
1045-1057: InitChainer unmarshals the entire app genesis into an EVM-only struct; InitGenesis gets the wrong payloadreq.AppStateBytes contains the full genesis map (module name → raw JSON). It must be unmarshaled into map[string]json.RawMessage and passed to ModuleManager.InitGenesis.
Apply this fix:
func (app *App) InitChainer(ctx sdk.Context, req *abci.RequestInitChain) (*abci.ResponseInitChain, error) { - var genesisState cosmosevmtypes.GenesisState - if err := json.Unmarshal(req.AppStateBytes, &genesisState); err != nil { + var genesisState map[string]json.RawMessage + if err := json.Unmarshal(req.AppStateBytes, &genesisState); err != nil { panic(err) } if err := app.UpgradeKeeper.SetModuleVersionMap(ctx, app.ModuleManager.GetVersionMap()); err != nil { panic(err) } - return app.ModuleManager.InitGenesis(ctx, app.appCodec, genesisState) + return app.ModuleManager.InitGenesis(ctx, app.appCodec, genesisState) }
🧹 Nitpick comments (12)
warden/app/oracle.go (3)
259-265: Recover + re-panic resets the original stack; log context before re-panickingThe current recover prints the current stack and then re-panics, which loses the original panic stack in the re-panic. Prefer logging rich context (height, proposer, etc.) and the debug.Stack() bytes, then re-panicking.
Apply this diff to improve observability:
- defer func() { - if err := recover(); err != nil { - debug.PrintStack() - panic(err) - } - }() + defer func() { + if r := recover(); r != nil { + // Log with context before re-panicking so we keep block info in logs. + stack := debug.Stack() + ctx.Logger().Error( + "panic during PreBlock", + "panic", r, + "height", ctx.BlockHeight(), + "stack", string(stack), + ) + panic(r) + } + }()Additionally, consider guarding against nil responses below when combining results:
- return &sdk.ResponsePreBlock{ - ConsensusParamsChanged: respA.ConsensusParamsChanged || respB.ConsensusParamsChanged, - }, nil + cpChanged := (respA != nil && respA.ConsensusParamsChanged) || (respB != nil && respB.ConsensusParamsChanged) + return &sdk.ResponsePreBlock{ConsensusParamsChanged: cpChanged}, nil
317-319: TxSelector is currently unused – remove or document its intended roleConfirmed via
rg -nP '\bTxSelector\b'thatTxSelectoronly appears at its declaration inwarden/app/oracle.go(lines 317–319), with no other references.Suggestions:
- If this stub is reserved for future wiring of custom transaction selectors, add a doc comment above it, for example:
+// TxSelector is a forward-compatibility stub for plugging in custom transaction selection logic. +// It will satisfy the `oracle.TxSelectorInterface` once that interface is defined. type TxSelector struct { App }- Otherwise, eliminate the type to prevent unneeded API surface:
-type TxSelector struct { - App -}
57-62: Document EVM Mempool Fallback inbaseProposalHandlerThe application’s EVM mempool is correctly wired during initialization (see
app.go:921–923, whereapp.SetMempool(evmMempool)is called), so passingnilintoNewDefaultProposalHandler(nil, app)will fall back to the EVM mempool rather than the SDK default. To make this explicit for future maintainers, please add a comment above the handler construction:func (app *App) baseProposalHandler() *baseapp.DefaultProposalHandler { - abciProposalHandler := baseapp.NewDefaultProposalHandler(nil, app) + // By passing nil here, DefaultProposalHandler will use the mempool set on the app + // (the EVM mempool is configured in app.go via app.SetMempool(evmMempool)) + abciProposalHandler := baseapp.NewDefaultProposalHandler(nil, app) abciProposalHandler.SetSignerExtractionAdapter( evmmempool.NewEthSignerExtractionAdapter( sdkmempool.NewDefaultSignerExtractionAdapter(), ), ) return abciProposalHandler }Alternatively (though not strictly necessary), you may pass
app.EVMMempoolexplicitly instead ofnilto remove any ambiguity.warden/x/sched/keeper/abci.go (1)
21-21: Alias go-ethereum crypto for clarity and to avoid stdlib confusionMinor readability nit: alias the go-ethereum crypto import (and the call site) to avoid ambiguity with the standard library packages and to make grepping easier.
Apply:
@@ - "github.com/ethereum/go-ethereum/crypto" + ethcrypto "github.com/ethereum/go-ethereum/crypto" @@ - tx.From = crypto.PubkeyToAddress(privkey.PublicKey).Bytes() + tx.From = ethcrypto.PubkeyToAddress(privkey.PublicKey).Bytes()Also applies to: 169-169
warden/app/precompiles.go (2)
40-76: Option pattern is solid; minor naming and shadowing nitsThe Optionals pattern is clean and extensible. Two small improvements:
- Rename Optionals to Options (conventional).
- Avoid parameter names that shadow imported package identifiers (e.g., codec).
Apply this diff:
-type Optionals struct { +type Options struct { @@ -func defaultOptionals() Optionals { - return Optionals{ +func defaultOptionals() Options { + return Options{ @@ -type Option func(opts *Optionals) +type Option func(opts *Options) @@ -func WithAddressCodec(codec address.Codec) Option { - return func(opts *Optionals) { - opts.AddressCodec = codec +func WithAddressCodec(addrCodec address.Codec) Option { + return func(opts *Options) { + opts.AddressCodec = addrCodec } } @@ -func WithValidatorAddrCodec(codec address.Codec) Option { - return func(opts *Optionals) { - opts.ValidatorAddrCodec = codec +func WithValidatorAddrCodec(valCodec address.Codec) Option { + return func(opts *Options) { + opts.ValidatorAddrCodec = valCodec } } @@ -func WithConsensusAddrCodec(codec address.Codec) Option { - return func(opts *Optionals) { - opts.ConsensusAddrCodec = codec +func WithConsensusAddrCodec(consCodec address.Codec) Option { + return func(opts *Options) { + opts.ConsensusAddrCodec = consCodec } }
154-161: Rename function parameter to avoid shadowing codec packageThe parameter codec codec.Codec shadows the imported "codec" package identifier, which is mildly confusing. Prefer cdc.
Apply this diff:
- codec codec.Codec, + cdc codec.Codec, @@ - govPrecompile, err := govprecompile.NewPrecompile( + govPrecompile, err := govprecompile.NewPrecompile( @@ - codec, + cdc,cmd/wardend/cmd/root.go (3)
100-109: Avoid shadowing initClientCtx inside PreRunEUsing := re-declares initClientCtx and can be confusing. Use = to keep a single variable in scope inside the closure.
Apply this diff:
- initClientCtx, err := client.ReadPersistentCommandFlags(initClientCtx, cmd.Flags()) + initClientCtx, err = client.ReadPersistentCommandFlags(initClientCtx, cmd.Flags())
155-163: AutoCLI config: redundant ReadFromClientConfigYou call ReadFromClientConfig inside PreRunE and again here. The second call is likely redundant.
Apply this diff to remove the duplicate invocation:
- initClientCtx, _ = clientcfg.ReadFromClientConfig(initClientCtx) - autoCliOpts.ClientCtx = initClientCtx + autoCliOpts.ClientCtx = initClientCtx
297-315: newApp: Prefer returning errors over panics where feasibleI realize the signature imposed by Cosmosevm server expects a constructor, but consider minimizing panics (e.g., parseEVMChainID failures) by validating earlier (PreRunE) and setting a sane default if missing. Not a blocker.
I can thread validated evmChainID via appOpts to avoid re-parsing here.
warden/app/app.go (2)
1425-1431: Avoid shadowing the imported url package with a local variable nameUsing url := url.Parse(...) shadows the imported package, which reduces readability and risks mistakes in future edits.
Apply a tiny rename:
- url, err := url.Parse(u) + parsedPricepredURL, err := url.Parse(u) if err != nil { panic(fmt.Errorf("invalid pricepred url: %s", u)) } - prophet.Register("pricepred", pricepred.NewPricePredictorSolidity(url)) + prophet.Register("pricepred", pricepred.NewPricePredictorSolidity(parsedPricepredURL))
1494-1501: Handle potential error from NewInterfaceRegistryWithOptions in NewTxConfigtypes.NewInterfaceRegistryWithOptions returns (InterfaceRegistry, error). You currently ignore the error; if the registry fails to build, returning a TxConfig will mask the root cause.
Consider:
- interfaceRegistry, _ := types.NewInterfaceRegistryWithOptions(types.InterfaceRegistryOptions{ + interfaceRegistry, err := types.NewInterfaceRegistryWithOptions(types.InterfaceRegistryOptions{ ProtoFiles: proto.HybridResolver, SigningOptions: signingOptions, }) + if err != nil { + panic(fmt.Errorf("failed to create interface registry: %w", err)) + } codec := codec.NewProtoCodec(interfaceRegistry) return authtx.NewTxConfig(codec, authtx.DefaultSignModes)justfile (1)
28-29: Parameterize gas/fees, emit raw values, and support both contract address keysHard-coding gas and fees can break across networks and testnets. Also, jq without -r returns quoted strings, which is awkward for reuse. Finally, some CosmWasm versions emit contract_address instead of _contract_address. Suggest the following targeted tweaks:
- Make gas/fees configurable via env with sane defaults.
- Use jq -r to return raw values.
- Accept either _contract_address or contract_address.
- Quote $CODE_ID during reuse.
- CODE_ID=$(wardend tx wasm store {{contract}} --from {{from}} -y --gas 10000000 --fees 1award | wardend q wait-tx -o json | jq '.events[] | select(.type == "store_code") | .attributes[] | select(.key == "code_id") | .value | tonumber') - ADDR=$(wardend tx wasm instantiate $CODE_ID '{}' --from {{from}} --label "{{if label == "" { "default" } else { label } }}" --no-admin --fees 1award -y | wardend q wait-tx -o json | jq '.events[] | select(.type == "instantiate") | .attributes[] | select(.key == "_contract_address") | .value') + CODE_ID=$(wardend tx wasm store {{contract}} --from {{from}} -y --gas "${GAS:-10000000}" --fees "${FEES:-1award}" \ + | wardend q wait-tx -o json \ + | jq -r '.events[] | select(.type=="store_code") | .attributes[] | select(.key=="code_id") | .value | tonumber') + ADDR=$(wardend tx wasm instantiate "${CODE_ID}" '{}' --from {{from}} --label "{{if label == "" { "default" } else { label } }}" --no-admin --fees "${FEES:-1award}" -y \ + | wardend q wait-tx -o json \ + | jq -r '.events[] | select(.type=="instantiate") | .attributes[] | select(.key=="_contract_address" or .key=="contract_address") | .value')Optional hardening outside the changed lines (no diff needed): after each assignment, fail fast if empty:
- test -n "$CODE_ID" || { echo "Failed to extract code id"; exit 1; }
- test -n "$ADDR" || { echo "Failed to extract contract address"; exit 1; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (64)
go.sumis excluded by!**/*.sumtests/testdata/snapshot-base/config/app.toml.tmplis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/genesis.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/gentx/gentx-38d79e1aae59e234ad6a60fd92896082bf41c64e.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/gentx/gentx-6945f3e30b02b9e34673acc22a18c3688ed57a19.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/node_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/priv_validator_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/keyring-test/80bd83f2b3f9f098fe4cd54891cdc86595a6d479.addressis excluded by!tests/testdata/**tests/testdata/snapshot-base/keyring-test/alice.infois excluded by!tests/testdata/**tests/testdata/snapshot-base/keyring-test/c8af84b4401237ec5048090f46c6327332be71cf.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/app.toml.tmplis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/genesis.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/gentx/gentx-32688592d17c81e774a966370c9366b35845614e.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/gentx/gentx-c1add3510774b379cfe92d361f1aa521dc9c9fc6.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/node_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/priv_validator_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/2ff01d371641069bd86371069b21a11f69383a33.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/32a224cecfe0a0660c93073fbcb6afebdbffa5ab.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/461cb598a46fcc43bef9044774afd0c4d66b54c9.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/63675504137926347258e5687f45cbf2b1f87b56.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/7daa0db7bba477374f17bf0cc2fa96d344bb8258.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/bob.infois excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/f114af4db343f425c8633979922e6f668f47dbc1.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/val.infois excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/writer.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/app.toml.tmplis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/genesis.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/gentx/gentx-2093ee4e76ca7a6460e5b92915bcbe61c8137495.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/gentx/gentx-a587c87739afa2c555eadf8b225cf7f389e31fa1.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/node_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/priv_validator_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/056d037ac7ae938ae179115f64a160d6976fc501.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/0fbc33e8e29268d962ebf0d9db19bf2715aee1d1.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/1856bab8f08b38ece497419342ec2ff2ba79b884.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/356e83fe984f8cff0e34868a9e239f7623d77644.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/460023f64738ff322a330aaadabe989057e72a66.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/640d841942f70598dea2ecc203be2efe0754fe37.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/6b8720d45650a5f4718bb85c0b3c713be7dabf83.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/alice.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/b730c0872fcb7106b5ab7fbaca574c4582e2a287.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/bddff3c30fabae236823a53b91505b5f445f973e.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/befc8528482269aef5c988845d0e365e1de01458.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/bf4bc4afeee16236f73202e7e93aafdab879c304.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/bob.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/charlie.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/dave.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/eb45ceb51c38527501b095031fa54c1b2911f981.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/erin.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/frank.infois excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/app.toml.tmplis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/genesis.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/gentx/gentx-2601681988711cb256c1068a6ae0fe35755fdb26.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/gentx/gentx-dbafff433fe3c3711b6ade63130895b84e777eb3.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/node_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/priv_validator_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/16159a91f2b78293bf6e1d0525433a6ad9fa7807.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/3005241c8fb90192342c9d58b86b434f8ba1d807.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/4596f61417e0c4fae43c201a9a0cd06e3b403ffd.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/79fce8f9a2fd6d67aba6a83f8d2b499c6b9a4eb1.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/alice.infois excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/b26f6dc33e0a93a55c3caa3dbf6e58155d6ca55f.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/bob.infois excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/dave.infois excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/e0ace9b0c67b860bd22cffcbb08591f3cc3b6e42.addressis excluded by!tests/testdata/**
📒 Files selected for processing (71)
CHANGELOG.md(1 hunks)cmd/wardend/cmd/commands.go(0 hunks)cmd/wardend/cmd/config.go(0 hunks)cmd/wardend/cmd/root.go(2 hunks)cmd/wardend/cmd/testnet.go(1 hunks)cmd/wardend/cmd/wait.go(0 hunks)cmd/wardend/config/chain_id.go(1 hunks)cmd/wardend/config/config.go(2 hunks)cmd/wardend/config/wardend_config.go(1 hunks)cmd/wardend/main.go(1 hunks)go.mod(9 hunks)justfile(1 hunks)localnet.just(3 hunks)precompiles/act/act.go(2 hunks)precompiles/async/async.go(2 hunks)precompiles/json/json.go(1 hunks)precompiles/precompiles.go(6 hunks)precompiles/sched/sched.go(2 hunks)precompiles/slinky/slinky.go(0 hunks)precompiles/warden/warden.go(6 hunks)prophet/registry.go(1 hunks)proto/warden/act/v1beta1/query.proto(0 hunks)tests/framework/exec/warden_eth.go(3 hunks)tests/framework/snapshot/snapshots.go(1 hunks)warden/app/ante.go(0 hunks)warden/app/ante/ante.go(1 hunks)warden/app/ante/cosmos.go(1 hunks)warden/app/ante/evm_handler.go(1 hunks)warden/app/ante/options.go(1 hunks)warden/app/ante/sched_handler.go(1 hunks)warden/app/app.go(4 hunks)warden/app/app_config.go(0 hunks)warden/app/config.go(1 hunks)warden/app/export.go(7 hunks)warden/app/legacy.go(0 hunks)warden/app/oracle.go(4 hunks)warden/app/precompiles.go(6 hunks)warden/app/sim_bench_test.go(0 hunks)warden/app/sim_test.go(0 hunks)warden/app/upgrades.go(1 hunks)warden/app/vemanager/vemanager.go(2 hunks)warden/testutil/network/network.go(0 hunks)warden/testutil/nullify/nullify.go(0 hunks)warden/testutil/sample/sample.go(0 hunks)warden/x/act/module/autocli.go(0 hunks)warden/x/act/module/genesis.go(0 hunks)warden/x/act/module/genesis_test.go(0 hunks)warden/x/act/module/simulation.go(1 hunks)warden/x/act/types/v1beta1/codec.go(0 hunks)warden/x/act/types/v1beta1/genesis.go(0 hunks)warden/x/act/types/v1beta1/genesis_test.go(1 hunks)warden/x/async/module/genesis.go(0 hunks)warden/x/async/module/genesis_test.go(0 hunks)warden/x/async/module/simulation.go(1 hunks)warden/x/async/types/v1beta1/genesis.go(0 hunks)warden/x/async/types/v1beta1/keys.go(1 hunks)warden/x/sched/keeper/abci.go(2 hunks)warden/x/sched/module/autocli.go(0 hunks)warden/x/sched/module/genesis.go(0 hunks)warden/x/sched/module/genesis_test.go(0 hunks)warden/x/sched/module/simulation.go(1 hunks)warden/x/sched/types/v1beta1/codec.go(0 hunks)warden/x/sched/types/v1beta1/genesis.go(0 hunks)warden/x/sched/types/v1beta1/genesis_test.go(1 hunks)warden/x/warden/module/autocli.go(0 hunks)warden/x/warden/module/genesis.go(0 hunks)warden/x/warden/module/genesis_test.go(0 hunks)warden/x/warden/module/simulation.go(1 hunks)warden/x/warden/types/v1beta3/codec.go(0 hunks)warden/x/warden/types/v1beta3/genesis.go(0 hunks)warden/x/warden/types/v1beta3/genesis_test.go(1 hunks)
💤 Files with no reviewable changes (31)
- warden/app/ante.go
- warden/x/warden/module/autocli.go
- warden/x/async/module/genesis.go
- warden/x/warden/module/genesis.go
- warden/app/sim_test.go
- warden/x/async/module/genesis_test.go
- warden/x/sched/types/v1beta1/codec.go
- warden/x/sched/module/genesis.go
- warden/x/sched/types/v1beta1/genesis.go
- warden/x/act/module/autocli.go
- warden/x/warden/types/v1beta3/genesis.go
- cmd/wardend/cmd/commands.go
- warden/testutil/network/network.go
- proto/warden/act/v1beta1/query.proto
- warden/x/act/types/v1beta1/genesis.go
- warden/app/legacy.go
- warden/x/act/module/genesis.go
- warden/x/sched/module/genesis_test.go
- warden/x/act/types/v1beta1/codec.go
- warden/testutil/nullify/nullify.go
- warden/x/async/types/v1beta1/genesis.go
- warden/app/app_config.go
- cmd/wardend/cmd/wait.go
- warden/testutil/sample/sample.go
- warden/x/act/module/genesis_test.go
- warden/x/sched/module/autocli.go
- warden/x/warden/module/genesis_test.go
- warden/x/warden/types/v1beta3/codec.go
- warden/app/sim_bench_test.go
- cmd/wardend/cmd/config.go
- precompiles/slinky/slinky.go
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (30)
- warden/x/sched/module/simulation.go
- warden/app/vemanager/vemanager.go
- tests/framework/snapshot/snapshots.go
- warden/x/sched/types/v1beta1/genesis_test.go
- warden/x/async/types/v1beta1/keys.go
- precompiles/sched/sched.go
- warden/app/ante/options.go
- cmd/wardend/config/chain_id.go
- precompiles/async/async.go
- precompiles/act/act.go
- precompiles/json/json.go
- warden/app/upgrades.go
- warden/x/act/module/simulation.go
- tests/framework/exec/warden_eth.go
- warden/app/ante/evm_handler.go
- cmd/wardend/cmd/testnet.go
- cmd/wardend/config/config.go
- warden/x/act/types/v1beta1/genesis_test.go
- warden/x/async/module/simulation.go
- warden/app/ante/sched_handler.go
- warden/app/ante/ante.go
- go.mod
- warden/app/ante/cosmos.go
- cmd/wardend/config/wardend_config.go
- precompiles/warden/warden.go
- warden/x/warden/module/simulation.go
- prophet/registry.go
- warden/x/warden/types/v1beta3/genesis_test.go
- cmd/wardend/main.go
- localnet.just
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-11-21T11:35:35.678Z
Learnt from: Pitasi
PR: warden-protocol/wardenprotocol#1095
File: warden/app/oracle.go:0-0
Timestamp: 2024-11-21T11:35:35.678Z
Learning: In the Go file `warden/app/oracle.go`, it is acceptable to panic inside the goroutine that starts the oracle client, because an error here indicates a misconfiguration (e.g., the address was misconfigured), and the application should exit.
Applied to files:
warden/app/oracle.go
📚 Learning: 2024-10-16T16:01:11.241Z
Learnt from: Pitasi
PR: warden-protocol/wardenprotocol#0
File: :0-0
Timestamp: 2024-10-16T16:01:11.241Z
Learning: Pitasi has implemented error handling in the `Expand` method of `warden/x/warden/keeper/shield.go`, replacing `panic` with error returns.
Applied to files:
cmd/wardend/cmd/root.go
📚 Learning: 2025-02-06T08:48:57.285Z
Learnt from: mn13
PR: warden-protocol/wardenprotocol#1216
File: warden/x/async/keeper/abci.go:212-216
Timestamp: 2025-02-06T08:48:57.285Z
Learning: In the collections package from Cosmos SDK, calling Keys() on an iterator fully consumes and closes the iterator, making an explicit Close() call unnecessary.
Applied to files:
warden/app/export.go
🧬 Code graph analysis (7)
warden/app/config.go (1)
warden/app/app.go (1)
App(183-254)
warden/app/oracle.go (1)
warden/app/app.go (1)
App(183-254)
cmd/wardend/cmd/root.go (4)
warden/app/app.go (2)
NewApp(256-1022)App(183-254)cmd/wardend/config/config.go (2)
EVMChainID(30-30)BaseDenom(26-26)cmd/wardend/config/wardend_config.go (2)
MustGetDefaultNodeHome(34-41)InitAppConfig(122-186)cmd/wardend/config/chain_id.go (1)
GetChainIDFromHome(12-29)
warden/app/export.go (1)
warden/app/app.go (1)
App(183-254)
precompiles/precompiles.go (6)
warden/x/async/types/v1beta1/expected_keepers.go (1)
BankKeeper(31-35)warden/x/sched/types/v1beta1/expected_keepers.go (1)
BankKeeper(18-22)precompiles/act/act.go (1)
NewPrecompile(45-70)precompiles/async/async.go (1)
NewPrecompile(45-70)precompiles/sched/sched.go (1)
NewPrecompile(46-73)precompiles/warden/warden.go (1)
NewPrecompile(50-78)
warden/app/precompiles.go (5)
precompiles/act/act.go (1)
NewPrecompile(45-70)precompiles/async/async.go (1)
NewPrecompile(45-70)precompiles/sched/sched.go (1)
NewPrecompile(46-73)precompiles/warden/warden.go (1)
NewPrecompile(50-78)precompiles/slinky/slinky.go (1)
NewPrecompile(47-70)
warden/app/app.go (8)
warden/x/act/keeper/keeper.go (2)
Keeper(21-38)NewKeeper(48-91)warden/x/async/keeper/keeper.go (2)
Keeper(34-61)NewKeeper(79-145)warden/x/warden/keeper/keeper.go (2)
Keeper(18-45)NewKeeper(62-122)warden/x/sched/keeper/keeper.go (2)
Keeper(22-34)NewKeeper(44-76)warden/app/oracle.go (1)
SlinkyClient(64-74)cmd/wardend/config/wardend_config.go (2)
GetMaccPerms(98-100)BlockedAddresses(69-95)warden/app/precompiles.go (1)
NewAvailableStaticPrecompiles(82-211)warden/app/ante/options.go (1)
HandlerOptions(15-23)
⏰ 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). (9)
- GitHub Check: lint
- GitHub Check: test
- GitHub Check: lint
- GitHub Check: test
- GitHub Check: test
- GitHub Check: lint
- GitHub Check: test
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (15)
warden/app/oracle.go (2)
3-17: LGTM on new imports for signer extraction and stack tracing
- Bringing in sdkmempool and evmmempool to wire the ETH signer extraction adapter is appropriate.
- runtime/debug is justified by the new recover block in combinePreBlocker.
107-120: Confirmed: panic on oracle client start error is acceptable hereThis aligns with our prior decision for warden/app/oracle.go: a failure to connect indicates misconfiguration and should abort startup. No change requested.
warden/x/sched/keeper/abci.go (1)
168-170: Derive and validate the sender from the signed tx; confirmFromEthereumTx()signatureWe ran a local search and confirmed:
- No local definition of
MsgEthereumTx.FromEthereumTxthat returns anerror.- No call sites in this repo expect or check an error from
FromEthereumTx.- All precompile methods authorize via
evm.Originorcontract.Caller(), nevertx.From, so overridingtx.Fromhas no immediate privilege impact.
⚠️ FromEthereumTxis implemented in the externalx/evm/typesmodule (pinned in yourgo.mod). Please manually verify that in the exact version you’ve vendored or tagged,FromEthereumTxstill does not return an error and that its behavior matches expectations.File:
warden/x/sched/keeper/abci.go(around lines 168–170)Suggested optional refactor:
- tx.FromEthereumTx(signedEthTx) - tx.From = crypto.PubkeyToAddress(privkey.PublicKey).Bytes() + tx.FromEthereumTx(signedEthTx) + signer := ethtypes.LatestSignerForChainID(signedEthTx.ChainId()) + sender, err := ethtypes.Sender(signer, signedEthTx) + if err != nil { + return nil, fmt.Errorf("derive sender from signedEthTx: %w", err) + } + // Defensive: ensure the derived sender matches the key we used to sign. + expected := crypto.PubkeyToAddress(privkey.PublicKey) + if sender != expected { + return nil, fmt.Errorf("sender mismatch: got %s, expected %s", sender.Hex(), expected.Hex()) + } + tx.From = sender.Bytes()warden/app/config.go (1)
24-29: Ensure codecs are initialized before calling eip712.SetEncodingConfigSetEncodingConfig uses app.legacyAmino and app.interfaceRegistry. If setupEVM is invoked before these are initialized, it will panic. Verify the call order in NewApp guarantees both are non-nil before setupEVM is called; alternatively, add a defensive check and return a descriptive error.
Would you like me to wire a small nil-check here with a unit test that fails when called too early?
warden/app/export.go (1)
239-254: LGTM: error propagation for signing infosThis section correctly accumulates iterErr via SetValidatorSigningInfo and propagates it after iteration.
warden/app/precompiles.go (2)
97-104: Initialize options: OK; verify bech32 prefixes are set before defaultOptionals()defaultOptionals pulls bech32 prefixes from sdk.GetConfig(). Ensure those prefixes are configured before NewAvailableStaticPrecompiles is called; otherwise codecs will be built with empty/incorrect prefixes.
If needed, I can add an assertion here to fail fast when prefixes are empty.
165-171: Slashing precompile wiring: LGTMValidator and consensus address codecs are injected via options; good separation of concerns.
precompiles/precompiles.go (1)
36-44: BankKeeper injection across warden precompiles: LGTMConstructor signatures and SetBalanceHandler usage look consistent. Centralized events registry is a nice touch.
Also applies to: 48-53, 65-71, 101-106, 121-127
cmd/wardend/cmd/root.go (5)
51-70: Temporary app bootstrap for encoding config: LGTMPre-instantiating the app to derive encoding, then cleaning up the temp directory, is a pragmatic approach.
110-128: Textual sign mode setup: LGTMConditionally enabling SIGN_MODE_TEXTUAL when online and wiring TextualCoinMetadataQueryFn is correct.
168-178: initTendermintConfig: LGTMReturning defaults with clearly commented optional overrides is fine.
180-234: Root command assembly: LGTMCommand composition, start options, and tx flags wiring look consistent with Cosmos EVM server patterns.
403-421: getChainIDFromOpts: LGTMFlag-first, fallback to client.toml from home is reasonable.
warden/app/app.go (1)
716-736: Ensure correct BankKeeper type for NewAvailableStaticPrecompilesThe third parameter of
NewAvailableStaticPrecompilesin warden/app/precompiles.go is declared asfunc NewAvailableStaticPrecompiles( stakingKeeper stakingkeeper.Keeper, distributionKeeper distributionkeeper.Keeper, bankKeeper cmn.BankKeeper, // … )Please confirm that
app.PreciseBankKeeper(passed on lines 716–736 of warden/app/app.go) satisfies thecmn.BankKeeperinterface. If it does not, replace it withapp.BankKeeper:app.EVMKeeper.WithStaticPrecompiles( NewAvailableStaticPrecompiles( *app.StakingKeeper, app.DistrKeeper, - app.PreciseBankKeeper, + app.BankKeeper, app.Erc20Keeper, app.TransferKeeper, app.IBCKeeper.ChannelKeeper, app.EVMKeeper, app.GovKeeper, app.SlashingKeeper, app.WardenKeeper, app.ActKeeper, app.OracleKeeper, app.AsyncKeeper, app.SchedKeeper, app.AppCodec(), ), )justfile (1)
27-27: Good call enabling strict bash modesAdding set -euxo pipefail will surface failures early, including in pipelines. Solid improvement for determinism in CI and local runs.
Pitasi
force-pushed
the
remove-depinject
branch
from
7ecd4da to
6d237cc
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
warden/x/warden/module/simulation.go (1)
25-28: Fix compile error: unused variable 'accs'.'accs' is declared and populated but never used; this will fail to compile.
Apply this diff to remove it:
- accs := make([]string, len(simState.Accounts)) - for i, acc := range simState.Accounts { - accs[i] = acc.Address.String() - }warden/app/config.go (1)
10-17: Global sealed prevents correct EVM config across multiple App instances; guard against chain-ID mismatchNewRootCmd spins a temporary App before the real node. With a package-level sealed, the real App may silently skip setupEVM and inherit the temp App’s chain config. At minimum, return an error if a different evmChainID is passed after sealing.
Apply:
@@ -import ( +import ( + "fmt" "cosmossdk.io/math" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/evm/ethereum/eip712" evmtypes "github.com/cosmos/evm/x/vm/types" ) @@ -var sealed = false +var sealed = false +var configuredEVMChainID uint64 @@ func (app *App) setupEVM(evmChainID uint64) error { if sealed { - return nil + if configuredEVMChainID != evmChainID { + return fmt.Errorf("EVM already configured with chain ID %d; requested %d", configuredEVMChainID, evmChainID) + } + return nil } @@ if err := evmtypes.NewEVMConfigurator(). WithChainConfig(ethCfg). WithEVMCoinInfo(coinInfo). Configure(); err != nil { return err } - sealed = true + configuredEVMChainID = evmChainID + sealed = trueAlso applies to: 19-25
warden/app/export.go (1)
61-71: Replace panic/log.Fatal with error returns in export pathsExport code should not terminate the process. Convert panics/log.Fatal to returned errors.
@@ - _, err := sdk.ValAddressFromBech32(addr) - if err != nil { - log.Fatal(err) - } + if _, err := sdk.ValAddressFromBech32(addr); err != nil { + return fmt.Errorf("invalid validator address %q: %w", addr, err) + } @@ - valAddr, err := sdk.ValAddressFromBech32(delegation.ValidatorAddress) - if err != nil { - panic(err) - } + valAddr, err := sdk.ValAddressFromBech32(delegation.ValidatorAddress) + if err != nil { + return fmt.Errorf("invalid validator address %q: %w", delegation.ValidatorAddress, err) + } @@ - if err := app.DistrKeeper.Hooks().BeforeDelegationCreated(ctx, delAddr, valAddr); err != nil { - // never called as BeforeDelegationCreated always returns nil - panic(fmt.Errorf("error while incrementing period: %w", err)) - } + if err := app.DistrKeeper.Hooks().BeforeDelegationCreated(ctx, delAddr, valAddr); err != nil { + return fmt.Errorf("error while incrementing period: %w", err) + } @@ - if err := app.DistrKeeper.Hooks().AfterDelegationModified(ctx, delAddr, valAddr); err != nil { - // never called as AfterDelegationModified always returns nil - panic(fmt.Errorf("error while creating a new delegation period record: %w", err)) - } + if err := app.DistrKeeper.Hooks().AfterDelegationModified(ctx, delAddr, valAddr); err != nil { + return fmt.Errorf("error while creating a new delegation period record: %w", err) + } @@ - _, err = app.StakingKeeper.ApplyAndReturnValidatorSetUpdates(ctx) - if err != nil { - log.Fatal(err) - } + if _, err = app.StakingKeeper.ApplyAndReturnValidatorSetUpdates(ctx); err != nil { + return err + }Also applies to: 89-97, 109-117, 229-233
precompiles/warden/warden.go (1)
105-116: Call balance hooks only for transactions (avoid state changes on queries).BeforeBalanceChange/AfterBalanceChange are invoked unconditionally, which can run on read-only queries. Guard them with IsTransaction(method).
Apply this diff:
func (p *Precompile) Run(evm *vm.EVM, contract *vm.Contract, readOnly bool) (bz []byte, err error) { - ctx, stateDB, method, initialGas, args, err := p.RunSetup(evm, contract, readOnly, p.IsTransaction) + ctx, stateDB, method, initialGas, args, err := p.RunSetup(evm, contract, readOnly, p.IsTransaction) if err != nil { return nil, err } - - p.GetBalanceHandler().BeforeBalanceChange(ctx) + isTx := p.IsTransaction(method) + if isTx { + p.GetBalanceHandler().BeforeBalanceChange(ctx) + } @@ - if err = p.GetBalanceHandler().AfterBalanceChange(ctx, stateDB); err != nil { - return nil, err - } + if isTx { + if err = p.GetBalanceHandler().AfterBalanceChange(ctx, stateDB); err != nil { + return nil, err + } + }Also applies to: 111-112, 188-193
♻️ Duplicate comments (4)
warden/app/export.go (2)
130-156: IterateValidators: closure errors are lost; capture and return iterErrErrors inside the closure stop iteration but are dropped. Capture them and return after IterateValidators.
-// reinitialize all validators -err = app.StakingKeeper.IterateValidators(ctx, func(_ int64, val stakingtypes.ValidatorI) (stop bool) { +// reinitialize all validators +var iterErr error +err = app.StakingKeeper.IterateValidators(ctx, func(_ int64, val stakingtypes.ValidatorI) (stop bool) { // donate any unwithdrawn outstanding reward fraction tokens to the community pool - scraps, err := app.DistrKeeper.GetValidatorOutstandingRewardsCoins(ctx, sdk.ValAddress(val.GetOperator())) - if err != nil { - return true - } + scraps, err := app.DistrKeeper.GetValidatorOutstandingRewardsCoins(ctx, sdk.ValAddress(val.GetOperator())) + if err != nil { iterErr = err; return true } - feePool, err := app.DistrKeeper.FeePool.Get(ctx) - if err != nil { - return true - } + feePool, err := app.DistrKeeper.FeePool.Get(ctx) + if err != nil { iterErr = err; return true } feePool.CommunityPool = feePool.CommunityPool.Add(scraps...) - err = app.DistrKeeper.FeePool.Set(ctx, feePool) - if err != nil { - return true - } + if err = app.DistrKeeper.FeePool.Set(ctx, feePool); err != nil { iterErr = err; return true } - err = app.DistrKeeper.Hooks().AfterValidatorCreated(ctx, sdk.ValAddress(val.GetOperator())) - // this lets us stop in case there's an error - return err != nil + if err = app.DistrKeeper.Hooks().AfterValidatorCreated(ctx, sdk.ValAddress(val.GetOperator())); err != nil { + iterErr = err + return true + } + return false }) if err != nil { return err } +if iterErr != nil { + return iterErr +}
225-228: Iterator Close error is swallowed; return itReturn the Close() error instead of logging and returning nil.
if err := iter.Close(); err != nil { app.Logger().Error("error while closing the key-value store reverse prefix iterator: ", err) - return nil + return fmt.Errorf("closing validator reverse iterator: %w", err) }warden/app/app.go (2)
912-927: EVMMempool captures AnteHandler before it is setMempool is initialized with AnteHandler before app.setAnteHandler runs, so it likely gets nil/old handler.
Move initialization to after setAnteHandler (and setPostHandler). Example:
- // mempool initialization (EVM priority nonce mempool) - if evmtypes.GetChainConfig() != nil { - // TODO: Get the actual block gas limit from consensus parameters - mempoolConfig := &evmmempool.EVMMempoolConfig{ - AnteHandler: app.GetAnteHandler(), - BlockGasLimit: 100_000_000, - } - evmMempool := evmmempool.NewExperimentalEVMMempool(app.CreateQueryContext, logger, app.EVMKeeper, app.FeeMarketKeeper, app.txConfig, app.clientCtx, mempoolConfig) - app.EVMMempool = evmMempool - app.SetMempool(evmMempool) - checkTxHandler := evmmempool.NewCheckTxHandler(evmMempool) - app.SetCheckTxHandler(checkTxHandler) - } @@ app.setAnteHandler(app.txConfig, nodeConfig, app.GetKey(wasmtypes.StoreKey), maxGasWanted) + // mempool initialization (EVM priority nonce mempool) + if evmtypes.GetChainConfig() != nil { + mempoolConfig := &evmmempool.EVMMempoolConfig{ + AnteHandler: app.GetAnteHandler(), + BlockGasLimit: 100_000_000, // TODO: derive from consensus params + } + evmMempool := evmmempool.NewExperimentalEVMMempool(app.CreateQueryContext, logger, app.EVMKeeper, app.FeeMarketKeeper, app.txConfig, app.clientCtx, mempoolConfig) + app.EVMMempool = evmMempool + app.SetMempool(evmMempool) + app.SetCheckTxHandler(evmmempool.NewCheckTxHandler(evmMempool)) + }Also applies to: 973-1000
1046-1058: InitChainer unmarshals genesis into wrong typereq.AppStateBytes is a map of module JSON. Unmarshal into map[string]json.RawMessage, not cosmosevmtypes.GenesisState.
-func (app *App) InitChainer(ctx sdk.Context, req *abci.RequestInitChain) (*abci.ResponseInitChain, error) { - var genesisState cosmosevmtypes.GenesisState +func (app *App) InitChainer(ctx sdk.Context, req *abci.RequestInitChain) (*abci.ResponseInitChain, error) { + var genesisState map[string]json.RawMessage if err := json.Unmarshal(req.AppStateBytes, &genesisState); err != nil { panic(err) } if err := app.UpgradeKeeper.SetModuleVersionMap(ctx, app.ModuleManager.GetVersionMap()); err != nil { panic(err) } return app.ModuleManager.InitGenesis(ctx, app.appCodec, genesisState) }
🧹 Nitpick comments (15)
prophet/registry.go (6)
3-6: Add sort import if we stabilize RegisteredPlugins output.If you adopt the sorting suggestion below, include sort in the imports.
Apply:
import ( - "fmt" - "sync" + "fmt" + "sort" + "sync" )
8-8: Update stale comment (“tasks plugins”).Change to “plugins” to match the rename.
-// registry is a global registry of tasks plugins. +// registry is a global registry of plugins.
13-16: Type name ‘r’ reduces readability.Consider a clearer type (e.g., registryT or pluginRegistry).
23-24: Duplicate registration handling: keep panic, add nil guard and simplify message.Also avoid creating an error just to panic; use Sprintf.
- if _, found := registry.plugins[name]; found { - panic(fmt.Errorf("plugin %s registered twice", name)) - } + if task == nil { + panic(fmt.Sprintf("plugin %q is nil", name)) + } + if _, found := registry.plugins[name]; found { + panic(fmt.Sprintf("plugin %q registered twice", name)) + }
41-46: Make RegisteredPlugins deterministic (sort names).Map iteration order is random; sorting removes flakiness in tests/CLI.
- plugins := make([]string, 0, len(registry.plugins)) + plugins := make([]string, 0, len(registry.plugins)) for name := range registry.plugins { plugins = append(plugins, name) } + sort.Strings(plugins) return pluginsRemember to add the sort import (see earlier diff).
30-35: Consider updating getPlugin to return a presence flagTo avoid callers mistaking a legitimately registered
nilplugin for “not found,” change the signature ofgetPlugininprophet/registry.goto return(Plugin, bool), and update its two call sites accordingly:• In prophet/registry.go
-func getPlugin(name string) Plugin { +func getPlugin(name string) (Plugin, bool) { registry.rw.RLock() defer registry.rw.RUnlock() - return registry.plugins[name] + p, ok := registry.plugins[name] + return p, ok }• In prophet/plugins.go, at the two call sites (around lines 30–31 and 66–67):
- s := getPlugin(f.Plugin) - if s == nil { + s, ok := getPlugin(f.Plugin) + if !ok { // handle missing plugin }Please update both call sites so they properly check the
okflag rather than relying ons == nil.warden/x/warden/module/simulation.go (3)
3-13: Remove leftover scaffolding imports and the sentinel var block.These imports and the "avoid unused import" block are no longer needed; trimming them reduces noise and avoids accidental dependencies.
import ( - "math/rand" - - sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" simtypes "github.com/cosmos/cosmos-sdk/types/simulation" - "github.com/cosmos/cosmos-sdk/x/simulation" - - wardensimulation "github.com/warden-protocol/wardenprotocol/warden/x/warden/simulation" types "github.com/warden-protocol/wardenprotocol/warden/x/warden/types/v1beta3" ) -// avoid unused import issue. -var ( - _ = wardensimulation.FindAccount - _ = rand.Rand{} - _ = sdk.AccAddress{} - _ = simulation.MsgEntryKind -)Also applies to: 15-21
45-49: Return nil instead of allocating an empty slice.No operations are registered; returning nil avoids a needless allocation and matches common SDK style.
func (am AppModule) WeightedOperations(simState module.SimulationState) []simtypes.WeightedOperation { - operations := make([]simtypes.WeightedOperation, 0) - - return operations + return nil }
51-54: Prefer nil slice for consistency with ProposalContents.Keep both governance helpers consistent and avoid an empty-slice allocation.
func (am AppModule) ProposalMsgs(simState module.SimulationState) []simtypes.WeightedProposalMsg { - return []simtypes.WeightedProposalMsg{} + return nil }warden/app/config.go (1)
24-35: Confirm eip712 encoding reconfiguration storysetupEVM configures eip712 with the passed chain ID. If the CLI later derives a different evmChainID (from flags/home), it must not be silently ignored. The above guard enforces this; alternatively, consider making sealing per-App (see app.go suggestion).
warden/app/export.go (1)
75-81: Handle reward withdrawal errors explicitly (optional)Ignore-result withdrawals can mask state issues. Consider propagating errors.
- _, _ = app.DistrKeeper.WithdrawValidatorCommission(ctx, sdk.ValAddress(val.GetOperator())) + if _, err := app.DistrKeeper.WithdrawValidatorCommission(ctx, sdk.ValAddress(val.GetOperator())); err != nil { + return fmt.Errorf("withdraw validator commission: %w", err) + } @@ - _, _ = app.DistrKeeper.WithdrawDelegationRewards(ctx, delAddr, valAddr) + if _, err := app.DistrKeeper.WithdrawDelegationRewards(ctx, delAddr, valAddr); err != nil { + return fmt.Errorf("withdraw delegation rewards: %w", err) + }Also applies to: 95-99
cmd/wardend/cmd/root.go (1)
157-160: Duplicate client config read (nit)initClientCtx is already read from client config in PersistentPreRunE. This extra read can be dropped unless needed for AutoCLI side-effects.
warden/app/app.go (2)
183-206: Make EVM encoding seal per-App (optional)To fully avoid cross-App leakage from tempApp, consider moving the seal state into App (e.g., fields evmEncodingSealed, evmEncodingChainID) and have setupEVM use those instead of package-level vars. This makes configuration robust under multiple App instances in one process.
Example field additions:
type App struct { *baseapp.BaseApp ... + // EVM encoding state (per-App) + evmEncodingSealed bool + evmEncodingChainID uint64 }And adjust setupEVM to read/write these fields.
915-919: Derive BlockGasLimit from consensus params (optional)Hardcoding 100_000_000 risks drift from network config. Read from app.GetConsensusParams(ctx) or server opts and plumb it here.
precompiles/warden/warden.go (1)
195-205: Outdated doc block.The “Available warden transactions are: -” comment is stale; either enumerate or remove.
Apply this diff:
-// Available warden transactions are: -// -// - +// Transactions: AddKeychainAdmin, AddKeychainWriter, FulfilKeyRequest, RejectKeyRequest, +// FulfilSignRequest, RejectSignRequest, NewKeychain, NewSpace, RemoveKeychainAdmin, +// UpdateKeychain, AddSpaceOwner, RemoveSpaceOwner, NewKeyRequest, NewSignRequest, +// UpdateKey, UpdateSpace.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (64)
go.sumis excluded by!**/*.sumtests/testdata/snapshot-base/config/app.toml.tmplis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/genesis.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/gentx/gentx-38d79e1aae59e234ad6a60fd92896082bf41c64e.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/gentx/gentx-6945f3e30b02b9e34673acc22a18c3688ed57a19.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/node_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/priv_validator_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/keyring-test/80bd83f2b3f9f098fe4cd54891cdc86595a6d479.addressis excluded by!tests/testdata/**tests/testdata/snapshot-base/keyring-test/alice.infois excluded by!tests/testdata/**tests/testdata/snapshot-base/keyring-test/c8af84b4401237ec5048090f46c6327332be71cf.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/app.toml.tmplis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/genesis.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/gentx/gentx-32688592d17c81e774a966370c9366b35845614e.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/gentx/gentx-c1add3510774b379cfe92d361f1aa521dc9c9fc6.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/node_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/priv_validator_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/2ff01d371641069bd86371069b21a11f69383a33.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/32a224cecfe0a0660c93073fbcb6afebdbffa5ab.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/461cb598a46fcc43bef9044774afd0c4d66b54c9.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/63675504137926347258e5687f45cbf2b1f87b56.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/7daa0db7bba477374f17bf0cc2fa96d344bb8258.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/bob.infois excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/f114af4db343f425c8633979922e6f668f47dbc1.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/val.infois excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/writer.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/app.toml.tmplis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/genesis.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/gentx/gentx-2093ee4e76ca7a6460e5b92915bcbe61c8137495.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/gentx/gentx-a587c87739afa2c555eadf8b225cf7f389e31fa1.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/node_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/priv_validator_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/056d037ac7ae938ae179115f64a160d6976fc501.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/0fbc33e8e29268d962ebf0d9db19bf2715aee1d1.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/1856bab8f08b38ece497419342ec2ff2ba79b884.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/356e83fe984f8cff0e34868a9e239f7623d77644.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/460023f64738ff322a330aaadabe989057e72a66.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/640d841942f70598dea2ecc203be2efe0754fe37.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/6b8720d45650a5f4718bb85c0b3c713be7dabf83.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/alice.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/b730c0872fcb7106b5ab7fbaca574c4582e2a287.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/bddff3c30fabae236823a53b91505b5f445f973e.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/befc8528482269aef5c988845d0e365e1de01458.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/bf4bc4afeee16236f73202e7e93aafdab879c304.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/bob.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/charlie.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/dave.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/eb45ceb51c38527501b095031fa54c1b2911f981.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/erin.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/frank.infois excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/app.toml.tmplis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/genesis.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/gentx/gentx-2601681988711cb256c1068a6ae0fe35755fdb26.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/gentx/gentx-dbafff433fe3c3711b6ade63130895b84e777eb3.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/node_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/priv_validator_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/16159a91f2b78293bf6e1d0525433a6ad9fa7807.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/3005241c8fb90192342c9d58b86b434f8ba1d807.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/4596f61417e0c4fae43c201a9a0cd06e3b403ffd.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/79fce8f9a2fd6d67aba6a83f8d2b499c6b9a4eb1.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/alice.infois excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/b26f6dc33e0a93a55c3caa3dbf6e58155d6ca55f.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/bob.infois excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/dave.infois excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/e0ace9b0c67b860bd22cffcbb08591f3cc3b6e42.addressis excluded by!tests/testdata/**
📒 Files selected for processing (71)
CHANGELOG.md(1 hunks)cmd/wardend/cmd/commands.go(0 hunks)cmd/wardend/cmd/config.go(0 hunks)cmd/wardend/cmd/root.go(2 hunks)cmd/wardend/cmd/testnet.go(1 hunks)cmd/wardend/cmd/wait.go(0 hunks)cmd/wardend/config/chain_id.go(1 hunks)cmd/wardend/config/config.go(2 hunks)cmd/wardend/config/wardend_config.go(1 hunks)cmd/wardend/main.go(1 hunks)go.mod(9 hunks)justfile(1 hunks)localnet.just(3 hunks)precompiles/act/act.go(2 hunks)precompiles/async/async.go(2 hunks)precompiles/json/json.go(1 hunks)precompiles/precompiles.go(6 hunks)precompiles/sched/sched.go(2 hunks)precompiles/slinky/slinky.go(0 hunks)precompiles/warden/warden.go(6 hunks)prophet/registry.go(1 hunks)proto/warden/act/v1beta1/query.proto(0 hunks)tests/framework/exec/warden_eth.go(3 hunks)tests/framework/snapshot/snapshots.go(1 hunks)warden/app/ante.go(0 hunks)warden/app/ante/ante.go(1 hunks)warden/app/ante/cosmos.go(1 hunks)warden/app/ante/evm_handler.go(1 hunks)warden/app/ante/options.go(1 hunks)warden/app/ante/sched_handler.go(1 hunks)warden/app/app.go(4 hunks)warden/app/app_config.go(0 hunks)warden/app/config.go(1 hunks)warden/app/export.go(7 hunks)warden/app/legacy.go(0 hunks)warden/app/oracle.go(4 hunks)warden/app/precompiles.go(6 hunks)warden/app/sim_bench_test.go(0 hunks)warden/app/sim_test.go(0 hunks)warden/app/upgrades.go(1 hunks)warden/app/vemanager/vemanager.go(2 hunks)warden/testutil/network/network.go(0 hunks)warden/testutil/nullify/nullify.go(0 hunks)warden/testutil/sample/sample.go(0 hunks)warden/x/act/module/autocli.go(0 hunks)warden/x/act/module/genesis.go(0 hunks)warden/x/act/module/genesis_test.go(0 hunks)warden/x/act/module/simulation.go(1 hunks)warden/x/act/types/v1beta1/codec.go(0 hunks)warden/x/act/types/v1beta1/genesis.go(0 hunks)warden/x/act/types/v1beta1/genesis_test.go(1 hunks)warden/x/async/module/genesis.go(0 hunks)warden/x/async/module/genesis_test.go(0 hunks)warden/x/async/module/simulation.go(1 hunks)warden/x/async/types/v1beta1/genesis.go(0 hunks)warden/x/async/types/v1beta1/keys.go(1 hunks)warden/x/sched/keeper/abci.go(2 hunks)warden/x/sched/module/autocli.go(0 hunks)warden/x/sched/module/genesis.go(0 hunks)warden/x/sched/module/genesis_test.go(0 hunks)warden/x/sched/module/simulation.go(1 hunks)warden/x/sched/types/v1beta1/codec.go(0 hunks)warden/x/sched/types/v1beta1/genesis.go(0 hunks)warden/x/sched/types/v1beta1/genesis_test.go(1 hunks)warden/x/warden/module/autocli.go(0 hunks)warden/x/warden/module/genesis.go(0 hunks)warden/x/warden/module/genesis_test.go(0 hunks)warden/x/warden/module/simulation.go(1 hunks)warden/x/warden/types/v1beta3/codec.go(0 hunks)warden/x/warden/types/v1beta3/genesis.go(0 hunks)warden/x/warden/types/v1beta3/genesis_test.go(1 hunks)
💤 Files with no reviewable changes (31)
- warden/app/sim_bench_test.go
- warden/x/warden/types/v1beta3/genesis.go
- warden/x/warden/types/v1beta3/codec.go
- warden/x/warden/module/autocli.go
- proto/warden/act/v1beta1/query.proto
- warden/x/async/types/v1beta1/genesis.go
- warden/x/sched/types/v1beta1/codec.go
- warden/x/sched/module/genesis_test.go
- warden/x/async/module/genesis.go
- warden/x/act/module/genesis_test.go
- warden/testutil/network/network.go
- warden/x/warden/module/genesis.go
- warden/x/sched/module/genesis.go
- warden/x/act/module/autocli.go
- warden/x/act/types/v1beta1/genesis.go
- warden/x/async/module/genesis_test.go
- warden/app/legacy.go
- warden/x/sched/types/v1beta1/genesis.go
- warden/testutil/nullify/nullify.go
- warden/testutil/sample/sample.go
- precompiles/slinky/slinky.go
- warden/x/sched/module/autocli.go
- warden/x/warden/module/genesis_test.go
- warden/app/app_config.go
- cmd/wardend/cmd/wait.go
- warden/app/sim_test.go
- cmd/wardend/cmd/commands.go
- warden/x/act/module/genesis.go
- warden/x/act/types/v1beta1/codec.go
- cmd/wardend/cmd/config.go
- warden/app/ante.go
🚧 Files skipped from review as they are similar to previous changes (31)
- warden/app/upgrades.go
- precompiles/sched/sched.go
- warden/x/act/types/v1beta1/genesis_test.go
- warden/app/ante/options.go
- warden/x/async/module/simulation.go
- cmd/wardend/config/wardend_config.go
- tests/framework/exec/warden_eth.go
- CHANGELOG.md
- tests/framework/snapshot/snapshots.go
- cmd/wardend/main.go
- warden/x/sched/types/v1beta1/genesis_test.go
- cmd/wardend/config/chain_id.go
- warden/x/async/types/v1beta1/keys.go
- warden/app/ante/cosmos.go
- precompiles/act/act.go
- warden/app/oracle.go
- warden/app/ante/evm_handler.go
- warden/x/sched/keeper/abci.go
- cmd/wardend/cmd/testnet.go
- cmd/wardend/config/config.go
- warden/x/sched/module/simulation.go
- warden/x/warden/types/v1beta3/genesis_test.go
- warden/app/ante/sched_handler.go
- warden/app/ante/ante.go
- warden/app/vemanager/vemanager.go
- precompiles/async/async.go
- warden/app/precompiles.go
- warden/x/act/module/simulation.go
- justfile
- go.mod
- localnet.just
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-10-16T16:01:11.241Z
Learnt from: Pitasi
PR: warden-protocol/wardenprotocol#0
File: :0-0
Timestamp: 2024-10-16T16:01:11.241Z
Learning: Pitasi has implemented error handling in the `Expand` method of `warden/x/warden/keeper/shield.go`, replacing `panic` with error returns.
Applied to files:
cmd/wardend/cmd/root.go
📚 Learning: 2024-11-02T10:21:03.096Z
Learnt from: backsapc
PR: warden-protocol/wardenprotocol#975
File: warden/app/app.go:562-562
Timestamp: 2024-11-02T10:21:03.096Z
Learning: In `warden/app/app.go`, suggestions to handle errors from `ParamsKeeper.GetSubspace()` may be considered out of scope.
Applied to files:
warden/app/app.go
📚 Learning: 2025-02-06T08:48:57.285Z
Learnt from: mn13
PR: warden-protocol/wardenprotocol#1216
File: warden/x/async/keeper/abci.go:212-216
Timestamp: 2025-02-06T08:48:57.285Z
Learning: In the collections package from Cosmos SDK, calling Keys() on an iterator fully consumes and closes the iterator, making an explicit Close() call unnecessary.
Applied to files:
warden/app/export.go
🧬 Code graph analysis (7)
cmd/wardend/cmd/root.go (4)
warden/app/app.go (2)
NewApp(256-1023)App(183-254)cmd/wardend/config/wardend_config.go (2)
MustGetDefaultNodeHome(34-41)InitAppConfig(122-186)cmd/wardend/cmd/testnet.go (1)
NewTestnetCmd(111-124)cmd/wardend/config/chain_id.go (1)
GetChainIDFromHome(12-29)
warden/app/config.go (1)
warden/app/app.go (1)
App(183-254)
precompiles/warden/warden.go (4)
precompiles/act/act.go (3)
Precompile(31-37)LoadABI(41-43)NewPrecompile(45-70)precompiles/async/async.go (3)
Precompile(31-37)LoadABI(41-43)NewPrecompile(45-70)precompiles/json/json.go (3)
Precompile(29-33)LoadABI(37-39)NewPrecompile(41-59)precompiles/sched/sched.go (3)
Precompile(31-38)LoadABI(42-44)NewPrecompile(46-73)
prophet/registry.go (1)
prophet/plugins.go (1)
Plugin(12-19)
precompiles/precompiles.go (6)
warden/x/async/types/v1beta1/expected_keepers.go (1)
BankKeeper(31-35)warden/x/sched/types/v1beta1/expected_keepers.go (1)
BankKeeper(18-22)precompiles/act/act.go (1)
NewPrecompile(45-70)precompiles/async/async.go (1)
NewPrecompile(45-70)precompiles/sched/sched.go (1)
NewPrecompile(46-73)precompiles/warden/warden.go (1)
NewPrecompile(50-78)
warden/app/app.go (8)
warden/x/act/keeper/keeper.go (2)
Keeper(21-38)NewKeeper(48-91)warden/x/async/keeper/keeper.go (2)
Keeper(34-61)NewKeeper(79-145)warden/x/warden/keeper/keeper.go (2)
Keeper(18-45)NewKeeper(62-122)warden/x/sched/keeper/keeper.go (2)
Keeper(22-34)NewKeeper(44-76)prophet/prophet.go (2)
P(22-31)New(34-51)warden/app/precompiles.go (1)
NewAvailableStaticPrecompiles(82-211)warden/app/ante/options.go (1)
HandlerOptions(15-23)warden/app/ante/ante.go (1)
NewAnteHandler(17-58)
warden/app/export.go (1)
warden/app/app.go (1)
App(183-254)
⏰ 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). (9)
- GitHub Check: test
- GitHub Check: lint
- GitHub Check: lint
- GitHub Check: test
- GitHub Check: test
- GitHub Check: lint
- GitHub Check: test
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (10)
prophet/registry.go (2)
10-11: Rename tasks → plugins — LGTM.Consistent with file intent; initialization is correct.
27-28: Write under Lock — LGTM.Safe write to the map under the mutex.
precompiles/json/json.go (1)
87-87: LGTM: stateDB intentionally unused in JSON precompile Run.JSON precompile is query-only (Build/Parse), so ignoring stateDB and dropping balance hooks is appropriate.
precompiles/warden/warden.go (3)
35-36: Constructor refactor looks good; BankKeeper wiring is correct.Embedding evmcmn.Precompile, switching LoadABI/HandleGasError to evmcmn, and calling SetBalanceHandler(bankKeeper) are consistent with the new pattern.
Also applies to: 47-48, 50-56, 62-67, 74-76
115-116: LGTM: switched to evmcmn.HandleGasError.Matches the dependency refactor and centralizes gas error handling.
118-176: Mixed use of evm.Origin vs contract.Caller() is intentional and consistent across precompilesAll methods in precompiles/warden/warden.go use only evm.Origin except the space- and request-related transactions, which correctly take both evm.Origin and contract.Caller():
- AddSpaceOwnerMethod, RemoveSpaceOwnerMethod, NewKeyRequestMethod, NewSignRequestMethod, UpdateKeyMethod, UpdateSpaceMethod
This mirrors the pattern in other precompiles (e.g., VoteForActionMethod in precompiles/act/act.go), so no changes are needed.
precompiles/precompiles.go (4)
3-8: LGTM: import evmcmn solely for BankKeeper.Keeps types explicit without leaking aliases elsewhere.
48-53: LGTM: BankKeeper plumbed into act/warden/async/sched constructors.Consistent with SetBalanceHandler in each precompile.
Also applies to: 65-70, 101-106, 121-121
114-120: Intentional: JSON precompile remains bank-agnostic.It’s query-only; no balance handler needed. Confirm no tx methods will be added here without revisiting this.
35-44: All call sites updated withbankKeeperI’ve verified that the only invocation of
NewWardenPrecompiles(inwarden/app/precompiles.goaround line 188) now passesbankKeeperas its first parameter. No other calls exist in the codebase, so the signature change has been fully propagated.
Pitasi
force-pushed
the
remove-depinject
branch
from
6d237cc to
12fedf7
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
warden/app/export.go (1)
61-71: Replace panic/log.Fatal with error returns in prepForZeroHeightGenesisThis function already returns error; panicking or exiting the process is hostile to callers and tooling.
Apply:
@@ - if err != nil { - log.Fatal(err) - } + if err != nil { + return fmt.Errorf("invalid jailed address %q: %w", addr, err) + } @@ - panic(err) + return fmt.Errorf("invalid validator address %q: %w", delegation.ValidatorAddress, err) @@ - panic(err) + return fmt.Errorf("invalid validator address %q: %w", del.ValidatorAddress, err) @@ - if err := app.DistrKeeper.Hooks().BeforeDelegationCreated(ctx, delAddr, valAddr); err != nil { - // never called as BeforeDelegationCreated always returns nil - panic(fmt.Errorf("error while incrementing period: %w", err)) - } + if err := app.DistrKeeper.Hooks().BeforeDelegationCreated(ctx, delAddr, valAddr); err != nil { + return fmt.Errorf("incrementing period: %w", err) + } @@ - if err := app.DistrKeeper.Hooks().AfterDelegationModified(ctx, delAddr, valAddr); err != nil { - // never called as AfterDelegationModified always returns nil - panic(fmt.Errorf("error while creating a new delegation period record: %w", err)) - } + if err := app.DistrKeeper.Hooks().AfterDelegationModified(ctx, delAddr, valAddr); err != nil { + return fmt.Errorf("creating new delegation period record: %w", err) + } @@ - if err != nil { - log.Fatal(err) - } + if err != nil { + return fmt.Errorf("apply validator set updates: %w", err) + }Also applies to: 90-117, 230-233
♻️ Duplicate comments (9)
justfile (1)
33-35: Avoid checksum drift: verify checksums match the resolved wasmvm_version (or pin both together)wasmvm_version is dynamic while checksums are hard-coded; they can diverge and break release images (used in lines 53–55 and 74–76). This was flagged earlier; please verify and gate CI on it.
Run from repo root:
#!/usr/bin/env bash set -euo pipefail # Extract expected checksums from justfile EXPECTED_AMD64="$(awk -F"'" '/wasmvm_amd64_checksum/ {print $2; exit}' justfile)" EXPECTED_ARM64="$(awk -F"'" '/wasmvm_arm64_checksum/ {print $2; exit}' justfile)" # Resolve wasmvm version (vX.Y.Z) via go, fallback to go.mod if command -v go >/dev/null 2>&1; then VER="$(go list -m -f '{{ .Version }}' github.com/CosmWasm/wasmvm/v3)" else VER="$(awk '/github.com\/CosmWasm\/wasmvm\/v3/ {print $2; exit}' go.mod)" fi [[ -n "${VER:-}" ]] tmpdir="$(mktemp -d)" trap 'rm -rf "$tmpdir"' EXIT curl -fsSL "https://github.com/CosmWasm/wasmvm/releases/download/${VER}/libwasmvm_muslc.x86_64.a" -o "${tmpdir}/libwasmvm_muslc.x86_64.a" curl -fsSL "https://github.com/CosmWasm/wasmvm/releases/download/${VER}/libwasmvm_muslc.aarch64.a" -o "${tmpdir}/libwasmvm_muslc.aarch64.a" AMD64_ACTUAL="$(sha256sum "${tmpdir}/libwasmvm_muslc.x86_64.a" | awk '{print $1}')" ARM64_ACTUAL="$(sha256sum "${tmpdir}/libwasmvm_muslc.aarch64.a" | awk '{print $1}')" echo "Resolved wasmvm version: ${VER}" echo "AMD64 expected: ${EXPECTED_AMD64}" echo "AMD64 actual : ${AMD64_ACTUAL}" echo "ARM64 expected: ${EXPECTED_ARM64}" echo "ARM64 actual : ${ARM64_ACTUAL}" [[ "${AMD64_ACTUAL}" == "${EXPECTED_AMD64}" && "${ARM64_ACTUAL}" == "${EXPECTED_ARM64}" ]] || { echo "❌ Checksum verification FAILED"; exit 1; } echo "✔️ Checksum verification PASSED"Optional hardening if you want local builds to work without Go installed:
-wasmvm_version := `go list -m -f '{{ .Version }}' github.com/CosmWasm/wasmvm/v3` +wasmvm_version := `sh -c 'go list -m -f "{{ .Version }}" github.com/CosmWasm/wasmvm/v3 2>/dev/null || awk "/github.com\\/CosmWasm\\/wasmvm\\/v3/ { print \\$2; exit }" go.mod'`warden/app/export.go (2)
130-156: Propagate closure errors from IterateValidators; they’re currently droppedErrors inside the iterator closure only stop iteration (return true) and never reach the caller. Capture them in an outer iterErr and return after iteration.
Apply:
@@ - // reinitialize all validators - err = app.StakingKeeper.IterateValidators(ctx, func(_ int64, val stakingtypes.ValidatorI) (stop bool) { + // reinitialize all validators + var iterErr error + err = app.StakingKeeper.IterateValidators(ctx, func(_ int64, val stakingtypes.ValidatorI) (stop bool) { @@ - scraps, err := app.DistrKeeper.GetValidatorOutstandingRewardsCoins(ctx, sdk.ValAddress(val.GetOperator())) - if err != nil { - return true - } + scraps, err := app.DistrKeeper.GetValidatorOutstandingRewardsCoins(ctx, sdk.ValAddress(val.GetOperator())) + if err != nil { iterErr = err; return true } @@ - feePool, err := app.DistrKeeper.FeePool.Get(ctx) - if err != nil { - return true - } + feePool, err := app.DistrKeeper.FeePool.Get(ctx) + if err != nil { iterErr = err; return true } @@ - err = app.DistrKeeper.FeePool.Set(ctx, feePool) - if err != nil { - return true - } + if err = app.DistrKeeper.FeePool.Set(ctx, feePool); err != nil { iterErr = err; return true } @@ - err = app.DistrKeeper.Hooks().AfterValidatorCreated(ctx, sdk.ValAddress(val.GetOperator())) - // this lets us stop in case there's an error - return err != nil + if err = app.DistrKeeper.Hooks().AfterValidatorCreated(ctx, sdk.ValAddress(val.GetOperator())); err != nil { + iterErr = err + return true + } + return false }) if err != nil { return err } +if iterErr != nil { + return iterErr +}
225-228: Do not swallow iterator Close error; return itLogging and returning nil hides real failures when closing the reverse iterator.
Apply:
if err := iter.Close(); err != nil { app.Logger().Error("error while closing the key-value store reverse prefix iterator: ", err) - return nil + return fmt.Errorf("closing validator reverse iterator: %w", err) }cmd/wardend/cmd/root.go (1)
61-71: Build tempApp with real EVM chain ID and close it to avoid config mismatches/leaksHard-coding EVMChainID here and not closing tempApp risks EIP-712 mismatches and sealed-global conflicts; also leaks resources.
Apply:
-noOpEvmAppOptions := func(_ uint64) error { return nil } -tempApp := app.NewApp( +evmForTemp := wardendconfig.EVMChainID +if home := wardendconfig.MustGetDefaultNodeHome(); home != "" { + if cid, err := wardendconfig.GetChainIDFromHome(home); err == nil { + if parsed, err := parseEVMChainID(cid); err == nil { + evmForTemp = parsed + } + } +} +tempApp := app.NewApp( log.NewNopLogger(), dbm.NewMemDB(), nil, true, simtestutil.NewAppOptionsWithFlagHome(temp), - wardendconfig.EVMChainID, - noOpEvmAppOptions, + evmForTemp, + wardendconfig.EvmAppOptions, ) +defer tempApp.Close()warden/app/app.go (3)
914-928: EVMMempool captures a nil AnteHandler due to orderingYou construct the EVM mempool before calling setAnteHandler, so AnteHandler in the mempool config is nil. This bypasses ante checks in CheckTx.
Apply (remove here; re-add after setAnteHandler):
-// mempool initialization (EVM priority nonce mempool) -if evmtypes.GetChainConfig() != nil { - // TODO: Get the actual block gas limit from consensus parameters - mempoolConfig := &evmmempool.EVMMempoolConfig{ - AnteHandler: app.GetAnteHandler(), - BlockGasLimit: 100_000_000, - } - - evmMempool := evmmempool.NewExperimentalEVMMempool(app.CreateQueryContext, logger, app.EVMKeeper, app.FeeMarketKeeper, app.txConfig, app.clientCtx, mempoolConfig) - app.EVMMempool = evmMempool - - app.SetMempool(evmMempool) - checkTxHandler := evmmempool.NewCheckTxHandler(evmMempool) - app.SetCheckTxHandler(checkTxHandler) -}
1049-1060: InitChainer unmarshals into the wrong type; breaks genesis initializationreq.AppStateBytes is the whole genesis map. Unmarshal into map[string]json.RawMessage and pass that to InitGenesis.
Apply:
func (app *App) InitChainer(ctx sdk.Context, req *abci.RequestInitChain) (*abci.ResponseInitChain, error) { - var genesisState cosmosevmtypes.GenesisState + var genesisState map[string]json.RawMessage if err := json.Unmarshal(req.AppStateBytes, &genesisState); err != nil { panic(err) } @@ - return app.ModuleManager.InitGenesis(ctx, app.appCodec, genesisState) + return app.ModuleManager.InitGenesis(ctx, app.appCodec, genesisState) }
981-1012: Initialize EVMMempool after ante/post handlers are setRecreate the mempool after SetAnteHandler/SetPostHandler so it captures the final handlers.
Apply:
@@ app.setPostHandler() + + // mempool initialization (EVM priority nonce mempool) + if evmtypes.GetChainConfig() != nil { + // TODO: derive from consensus params + mempoolConfig := &evmmempool.EVMMempoolConfig{ + AnteHandler: app.GetAnteHandler(), + BlockGasLimit: 100_000_000, + } + evmMempool := evmmempool.NewExperimentalEVMMempool( + app.CreateQueryContext, logger, app.EVMKeeper, app.FeeMarketKeeper, app.txConfig, app.clientCtx, mempoolConfig, + ) + app.EVMMempool = evmMempool + app.SetMempool(evmMempool) + app.SetCheckTxHandler(evmmempool.NewCheckTxHandler(evmMempool)) + }cmd/wardend/cmd/testnet.go (2)
558-569: Fix: invalid loop and last-octet overflow in calculateIP.The current loop won’t compile and may wrap the last octet. Do a bounded addition.
func calculateIP(ip string, i int) (string, error) { ipv4 := net.ParseIP(ip).To4() if ipv4 == nil { return "", fmt.Errorf("%v: non ipv4 address", ip) } - for range i { - ipv4[3]++ - } - - return ipv4.String(), nil + octet := int(ipv4[3]) + i + if octet < 0 || octet > 255 { + return "", fmt.Errorf("IP address overflow: starting IP %s with offset %d exceeds last-octet range 0-255", ip, i) + } + ipv4[3] = byte(octet) + return ipv4.String(), nil }
236-243: Add bounds check for validator count (fail fast).Prevents accidental resource blowups and aligns with earlier feedback.
if args.chainID == "" { args.chainID = "local-4221" } + + // sanity-check number of validators + if args.numValidators < 1 || args.numValidators > 100 { + return fmt.Errorf("invalid --%s: %d (must be between 1 and 100)", flagNumValidators, args.numValidators) + }
🧹 Nitpick comments (24)
justfile (1)
27-29: Harden deploy-contract: use raw jq output and tolerate SDK event key changes
- Add jq -r to avoid quoted values in CODE_ID/ADDR.
- Make address extraction resilient to either _contract_address or contract_address.
- CODE_ID=$(wardend tx wasm store {{contract}} --from {{from}} -y --gas 10000000 --fees 1award | wardend q wait-tx -o json | jq '.events[] | select(.type == "store_code") | .attributes[] | select(.key == "code_id") | .value | tonumber') - ADDR=$(wardend tx wasm instantiate $CODE_ID '{}' --from {{from}} --label "{{if label == "" { "default" } else { label } }}" --no-admin --fees 1award -y | wardend q wait-tx -o json | jq '.events[] | select(.type == "instantiate") | .attributes[] | select(.key == "_contract_address") | .value') + CODE_ID=$(wardend tx wasm store {{contract}} --from {{from}} -y --gas 10000000 --fees 1award \ + | wardend q wait-tx -o json \ + | jq -r '.events[] | select(.type == "store_code") | .attributes[] | select(.key == "code_id") | .value | tonumber') + ADDR=$(wardend tx wasm instantiate $CODE_ID '{}' --from {{from}} --label "{{if label == "" { "default" } else { label } }}" --no-admin --fees 1award -y \ + | wardend q wait-tx -o json \ + | jq -r '.events[] | select(.type == "instantiate") | .attributes[] | select(.key == "_contract_address" or .key == "contract_address") | .value' | head -n1)Optional: parameterize gas/fees so the recipe works across chains and fee regimes:
- deploy-contract contract from="shulgin" label="": + deploy-contract contract from="shulgin" label="" gas="10000000" fees="1award":and then replace the two occurrences of
--gas 10000000 --fees 1awardwith--gas {{gas}} --fees {{fees}}.cmd/wardend/config/evm_app_options.go (2)
16-26: Use sync.Once instead of a package-level sealed bool.Avoid races if init is called twice (tests/CLI).
Apply:
- var sealed bool + // guarded one-time EVM options init + var evmAppOptionsOnce sync.Once-func EvmAppOptions(evmChainID uint64) error { - if sealed { - return nil - } - - sealed = true +func EvmAppOptions(evmChainID uint64) error { + var err error + evmAppOptionsOnce.Do(func() { + // set the denom info for the chain + if e := setBaseDenom(coinInfo); e != nil { + err = e + return + } + + ethCfg := evmtypes.DefaultChainConfig(evmChainID) + if e := evmtypes.NewEVMConfigurator(). + WithChainConfig(ethCfg). + WithEVMCoinInfo(coinInfo). + Configure(); e != nil { + err = e + return + } + }) + return err - // set the denom info for the chain - if err := setBaseDenom(coinInfo); err != nil { - return err - } - - ethCfg := evmtypes.DefaultChainConfig(evmChainID) - if err := evmtypes.NewEVMConfigurator(). - WithChainConfig(ethCfg). - WithEVMCoinInfo(coinInfo). - Configure(); err != nil { - return err - } - - return nil }Additional change outside this hunk:
- add import:
sync
50-52: Comment is misleading.It’s not “overwrite the base denom”; we’re registering scale factors for two denoms. Suggest clarifying or remove.
warden/app/precompiles.go (1)
97-99: Avoid shadowing the imported codec package with a param named codec.Rename parameter to cdc for clarity.
Apply:
-func NewAvailableStaticPrecompiles( +func NewAvailableStaticPrecompiles( ... - codec codec.Codec, + cdc codec.Codec, opts ...Option, ) map[common.Address]vm.PrecompiledContract {- govPrecompile, err := govprecompile.NewPrecompile( + govPrecompile, err := govprecompile.NewPrecompile( govkeeper.NewMsgServerImpl(&govKeeper), govkeeper.NewQueryServer(&govKeeper), bankKeeper, - codec, + cdc, options.AddressCodec, )Also applies to: 154-160
precompiles/precompiles.go (1)
35-36: Consider updating the function comment to mention BankKeeper injection.Minor doc touch-up for future readers.
warden/app/export.go (2)
164-166: Avoid redeclaring iterErr; reuse the outer variableOnce you introduce iterErr above, remove this duplicate declaration to prevent shadowing.
Apply:
-var iterErr error
75-81: Ignored reward-withdrawal errors; at least surface or aggregate themSilent failures during withdrawals may leave distribution state inconsistent. Consider capturing and returning the first error (or aggregating).
Example:
-_, _ = app.DistrKeeper.WithdrawValidatorCommission(ctx, sdk.ValAddress(val.GetOperator())) +if _, err := app.DistrKeeper.WithdrawValidatorCommission(ctx, sdk.ValAddress(val.GetOperator())); err != nil { + return fmt.Errorf("withdraw validator commission %s: %w", val.GetOperator(), err) +} @@ -_, _ = app.DistrKeeper.WithdrawDelegationRewards(ctx, delAddr, valAddr) +if _, err := app.DistrKeeper.WithdrawDelegationRewards(ctx, delAddr, valAddr); err != nil { + return fmt.Errorf("withdraw delegation rewards %s->%s: %w", delAddr, valAddr, err) +}Also applies to: 95-99
cmd/wardend/cmd/root.go (2)
153-155: Handle error from ReadFromClientConfig instead of discarding itDiscarding this error can mask misconfigurations that later cause panics.
Apply:
-initClientCtx, _ = clientcfg.ReadFromClientConfig(initClientCtx) +if cctx, err := clientcfg.ReadFromClientConfig(initClientCtx); err == nil { + initClientCtx = cctx +}
160-169: Avoid panics on ChainID parsing/EVM options; log and continuePanicking in NewRootCmd aborts the binary during CLI assembly. Prefer graceful degradation.
Apply:
if initClientCtx.ChainID != "" { - evmChainID, err := parseEVMChainID(initClientCtx.ChainID) - if err != nil { - panic(err) - } - - if err := wardendconfig.EvmAppOptions(evmChainID); err != nil { - panic(err) - } + if evmChainID, err := parseEVMChainID(initClientCtx.ChainID); err == nil { + _ = wardendconfig.EvmAppOptions(evmChainID) + } else { + fmt.Fprintf(os.Stderr, "warning: failed to configure EVM options for ChainID %q: %v\n", initClientCtx.ChainID, err) + } }prophet/registry.go (5)
3-6: Deterministic plugin listing: import sort to enable ordered output.
If we sort RegisteredPlugins, we’ll need this import.import ( "fmt" + "sort" "sync" )
14-15: Nit: clarify type name and consider embedding the mutex.
Renaming r to pluginRegistry (and embedding sync.RWMutex) improves readability.Example:
type pluginRegistry struct { sync.RWMutex plugins map[string]Plugin }
23-24: Nit: panic with formatted string and quote the name.
Avoid panicking with an error value; quote the plugin name for clarity.- panic(fmt.Errorf("plugin %s registered twice", name)) + panic(fmt.Sprintf("plugin %q registered twice", name))
18-28: Rename parameter from task to plugin and fix doc comment.
Aligns naming with the map and public API; avoids mixed “task/plugin” terminology.-// Register registers a new task plugin with the given unique name. +// Register registers a new plugin with the given unique name. -func Register(name string, task Plugin) { +func Register(name string, plugin Plugin) { registry.rw.Lock() defer registry.rw.Unlock() if _, found := registry.plugins[name]; found { panic(fmt.Errorf("plugin %s registered twice", name)) } - registry.plugins[name] = task + registry.plugins[name] = plugin }
37-47: Return plugin names in deterministic order and avoid holding the lock while sorting.
Stable order helps tests/config diffs; unlock before sort to reduce lock hold time. Add a doc comment for the exported function.+// RegisteredPlugins returns the names of all registered plugins in lexicographic order. func RegisteredPlugins() []string { - registry.rw.RLock() - defer registry.rw.RUnlock() - - plugins := make([]string, 0, len(registry.plugins)) - for name := range registry.plugins { - plugins = append(plugins, name) - } - - return plugins + registry.rw.RLock() + names := make([]string, 0, len(registry.plugins)) + for name := range registry.plugins { + names = append(names, name) + } + registry.rw.RUnlock() + + sort.Strings(names) + return names }Note: This requires importing "sort" (see earlier comment).
cmd/wardend/cmd/testnet.go (10)
90-105: CLI polish: add -v alias and fix gas price help example.Small UX/doc wins.
- cmd.Flags().Int(flagNumValidators, 4, "Number of validators to initialize the testnet with") + cmd.Flags().IntP(flagNumValidators, "v", 4, "Number of validators to initialize the testnet with") @@ - cmd.Flags().String(server.FlagMinGasPrices, "0.000006"+sdk.DefaultBondDenom, "Minimum gas prices to accept for transactions; All fees in a tx must meet this minimum (e.g. 0.01photino,0.001stake)") + cmd.Flags().String(server.FlagMinGasPrices, "0.000006"+sdk.DefaultBondDenom, "Minimum gas prices to accept for transactions; all fees must meet this minimum (e.g. 0.01stake,0.001testtoken)")
131-142: Fix examples: binary name and flag usage.Docs say “evmd” and “--v”; CLI here is wardend and you added -v/--validator-count.
-Example: - evmd testnet init-files --v 4 --output-dir ./.testnets --starting-ip-address 192.168.10.2 +Example: + wardend testnet init-files -v 4 --output-dir ./.testnets --starting-ip-address 192.168.10.2
195-197: Fix examples: command and flag.-Example: - evmd testnet --v 4 --output-dir ./.testnets +Example: + wardend testnet start -v 4 --output-dir ./.testnets
179-179: Correct port in help text (26656, not 46656).- cmd.Flags().String(flagStartingIPAddress, "192.168.0.1", "Starting IP address (192.168.0.1 results in persistent peers list [email protected]:46656, [email protected]:46656, ...)") + cmd.Flags().String(flagStartingIPAddress, "192.168.0.1", "Starting IP address (192.168.0.1 results in persistent peers list [email protected]:26656, [email protected]:26656, ...)")
605-611: Use filepath.Join and fix “testnests” typo in error.- baseDir := fmt.Sprintf("%s/%s", args.outputDir, networkConfig.ChainID) + baseDir := filepath.Join(args.outputDir, networkConfig.ChainID) if _, err := os.Stat(baseDir); !os.IsNotExist(err) { return fmt.Errorf( - "testnests directory already exists for chain-id '%s': %s, please remove or select a new --chain-id", + "testnets directory already exists for chain-id '%s': %s, please remove or select a new --chain-id", networkConfig.ChainID, baseDir) }
612-629: Defer cleanup and avoid fmt.Scanln() with no args.Ensures cleanup even on early return; avoid Scanln pitfalls.
- testnet, err := network.New(networkLogger, baseDir, networkConfig) + testnet, err := network.New(networkLogger, baseDir, networkConfig) if err != nil { return err } + defer testnet.Cleanup() @@ - cmd.Println("press the Enter Key to terminate") - - if _, err := fmt.Scanln(); err != nil { // wait for Enter Key - return err - } - - testnet.Cleanup() + cmd.Println("press the Enter key to terminate") + _, _ = bufio.NewReader(os.Stdin).ReadString('\n') return nil
178-178: Optional: default node home to “wardend” for consistency.Matches the binary name; change only if it won’t break existing workflows.
- cmd.Flags().String(flagNodeDaemonHome, "evmd", "Home directory of the node's daemon configuration") + cmd.Flags().String(flagNodeDaemonHome, "wardend", "Home directory of the node's daemon configuration")
75-75: Unused flag: --use-docker (and associated arg).Flag is parsed but never used. Wire it up or drop it to avoid confusion.
Also applies to: 181-182, 157-158
632-639: Nit: remove temp dir creation that isn’t used.The app uses MemDB and no home dir; the temp dir is vestigial.
func NewTestNetworkFixture() network.TestFixture { - dir, err := os.MkdirTemp("", "evm") - if err != nil { - panic(fmt.Sprintf("failed creating temporary directory: %v", err)) - } - defer os.RemoveAll(dir) - app := evmd.NewExampleApp( log.NewNopLogger(), dbm.NewMemDB(), nil,
131-141: Doc comment above NewTestnetCmd mentions 3 subcommands; only 2 exist.Trim or update the comment to reflect current commands.
Also applies to: 195-197
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (64)
go.sumis excluded by!**/*.sumtests/testdata/snapshot-base/config/app.toml.tmplis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/genesis.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/gentx/gentx-38d79e1aae59e234ad6a60fd92896082bf41c64e.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/gentx/gentx-6945f3e30b02b9e34673acc22a18c3688ed57a19.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/node_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/priv_validator_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/keyring-test/80bd83f2b3f9f098fe4cd54891cdc86595a6d479.addressis excluded by!tests/testdata/**tests/testdata/snapshot-base/keyring-test/alice.infois excluded by!tests/testdata/**tests/testdata/snapshot-base/keyring-test/c8af84b4401237ec5048090f46c6327332be71cf.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/app.toml.tmplis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/genesis.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/gentx/gentx-32688592d17c81e774a966370c9366b35845614e.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/gentx/gentx-c1add3510774b379cfe92d361f1aa521dc9c9fc6.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/node_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/priv_validator_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/2ff01d371641069bd86371069b21a11f69383a33.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/32a224cecfe0a0660c93073fbcb6afebdbffa5ab.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/461cb598a46fcc43bef9044774afd0c4d66b54c9.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/63675504137926347258e5687f45cbf2b1f87b56.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/7daa0db7bba477374f17bf0cc2fa96d344bb8258.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/bob.infois excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/f114af4db343f425c8633979922e6f668f47dbc1.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/val.infois excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/writer.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/app.toml.tmplis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/genesis.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/gentx/gentx-2093ee4e76ca7a6460e5b92915bcbe61c8137495.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/gentx/gentx-a587c87739afa2c555eadf8b225cf7f389e31fa1.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/node_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/priv_validator_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/056d037ac7ae938ae179115f64a160d6976fc501.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/0fbc33e8e29268d962ebf0d9db19bf2715aee1d1.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/1856bab8f08b38ece497419342ec2ff2ba79b884.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/356e83fe984f8cff0e34868a9e239f7623d77644.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/460023f64738ff322a330aaadabe989057e72a66.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/640d841942f70598dea2ecc203be2efe0754fe37.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/6b8720d45650a5f4718bb85c0b3c713be7dabf83.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/alice.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/b730c0872fcb7106b5ab7fbaca574c4582e2a287.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/bddff3c30fabae236823a53b91505b5f445f973e.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/befc8528482269aef5c988845d0e365e1de01458.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/bf4bc4afeee16236f73202e7e93aafdab879c304.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/bob.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/charlie.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/dave.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/eb45ceb51c38527501b095031fa54c1b2911f981.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/erin.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/frank.infois excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/app.toml.tmplis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/genesis.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/gentx/gentx-2601681988711cb256c1068a6ae0fe35755fdb26.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/gentx/gentx-dbafff433fe3c3711b6ade63130895b84e777eb3.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/node_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/priv_validator_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/16159a91f2b78293bf6e1d0525433a6ad9fa7807.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/3005241c8fb90192342c9d58b86b434f8ba1d807.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/4596f61417e0c4fae43c201a9a0cd06e3b403ffd.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/79fce8f9a2fd6d67aba6a83f8d2b499c6b9a4eb1.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/alice.infois excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/b26f6dc33e0a93a55c3caa3dbf6e58155d6ca55f.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/bob.infois excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/dave.infois excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/e0ace9b0c67b860bd22cffcbb08591f3cc3b6e42.addressis excluded by!tests/testdata/**
📒 Files selected for processing (73)
CHANGELOG.md(1 hunks)cmd/wardend/cmd/commands.go(0 hunks)cmd/wardend/cmd/config.go(0 hunks)cmd/wardend/cmd/root.go(1 hunks)cmd/wardend/cmd/testnet.go(1 hunks)cmd/wardend/cmd/wait.go(0 hunks)cmd/wardend/config/chain_id.go(1 hunks)cmd/wardend/config/config.go(2 hunks)cmd/wardend/config/evm_app_options.go(1 hunks)cmd/wardend/config/wardend_config.go(1 hunks)cmd/wardend/main.go(1 hunks)go.mod(9 hunks)justfile(1 hunks)localnet.just(3 hunks)precompiles/act/act.go(2 hunks)precompiles/async/async.go(2 hunks)precompiles/json/json.go(1 hunks)precompiles/precompiles.go(6 hunks)precompiles/sched/sched.go(2 hunks)precompiles/slinky/slinky.go(0 hunks)precompiles/warden/warden.go(6 hunks)prophet/registry.go(1 hunks)proto/warden/act/v1beta1/query.proto(0 hunks)tests/framework/exec/warden_eth.go(3 hunks)tests/framework/snapshot/snapshots.go(1 hunks)warden/app/ante.go(0 hunks)warden/app/ante/ante.go(1 hunks)warden/app/ante/cosmos.go(1 hunks)warden/app/ante/evm_handler.go(1 hunks)warden/app/ante/options.go(1 hunks)warden/app/ante/sched_handler.go(1 hunks)warden/app/app.go(4 hunks)warden/app/app_config.go(0 hunks)warden/app/config.go(0 hunks)warden/app/export.go(7 hunks)warden/app/genesis.go(0 hunks)warden/app/legacy.go(0 hunks)warden/app/oracle.go(4 hunks)warden/app/precompiles.go(6 hunks)warden/app/sim_bench_test.go(0 hunks)warden/app/sim_test.go(0 hunks)warden/app/upgrades.go(1 hunks)warden/app/vemanager/vemanager.go(2 hunks)warden/testutil/network/network.go(0 hunks)warden/testutil/nullify/nullify.go(0 hunks)warden/testutil/sample/sample.go(0 hunks)warden/x/act/module/autocli.go(0 hunks)warden/x/act/module/genesis.go(0 hunks)warden/x/act/module/genesis_test.go(0 hunks)warden/x/act/module/simulation.go(1 hunks)warden/x/act/types/v1beta1/codec.go(0 hunks)warden/x/act/types/v1beta1/genesis.go(0 hunks)warden/x/act/types/v1beta1/genesis_test.go(1 hunks)warden/x/async/module/genesis.go(0 hunks)warden/x/async/module/genesis_test.go(0 hunks)warden/x/async/module/simulation.go(1 hunks)warden/x/async/types/v1beta1/genesis.go(0 hunks)warden/x/async/types/v1beta1/keys.go(1 hunks)warden/x/sched/keeper/abci.go(2 hunks)warden/x/sched/module/autocli.go(0 hunks)warden/x/sched/module/genesis.go(0 hunks)warden/x/sched/module/genesis_test.go(0 hunks)warden/x/sched/module/simulation.go(1 hunks)warden/x/sched/types/v1beta1/codec.go(0 hunks)warden/x/sched/types/v1beta1/genesis.go(0 hunks)warden/x/sched/types/v1beta1/genesis_test.go(1 hunks)warden/x/warden/module/autocli.go(0 hunks)warden/x/warden/module/genesis.go(0 hunks)warden/x/warden/module/genesis_test.go(0 hunks)warden/x/warden/module/simulation.go(1 hunks)warden/x/warden/types/v1beta3/codec.go(0 hunks)warden/x/warden/types/v1beta3/genesis.go(0 hunks)warden/x/warden/types/v1beta3/genesis_test.go(1 hunks)
💤 Files with no reviewable changes (33)
- warden/x/sched/types/v1beta1/genesis.go
- warden/x/sched/types/v1beta1/codec.go
- warden/x/async/types/v1beta1/genesis.go
- cmd/wardend/cmd/wait.go
- precompiles/slinky/slinky.go
- warden/x/sched/module/genesis.go
- warden/x/async/module/genesis.go
- warden/x/warden/types/v1beta3/genesis.go
- warden/x/warden/module/genesis.go
- cmd/wardend/cmd/config.go
- warden/x/async/module/genesis_test.go
- warden/x/sched/module/autocli.go
- warden/x/act/types/v1beta1/genesis.go
- warden/x/warden/types/v1beta3/codec.go
- warden/testutil/sample/sample.go
- warden/x/warden/module/genesis_test.go
- warden/app/genesis.go
- warden/x/act/module/autocli.go
- warden/app/sim_test.go
- warden/testutil/nullify/nullify.go
- warden/app/config.go
- warden/app/app_config.go
- warden/x/sched/module/genesis_test.go
- warden/app/sim_bench_test.go
- warden/app/ante.go
- warden/x/act/module/genesis.go
- proto/warden/act/v1beta1/query.proto
- warden/x/act/module/genesis_test.go
- warden/x/warden/module/autocli.go
- warden/testutil/network/network.go
- warden/x/act/types/v1beta1/codec.go
- warden/app/legacy.go
- cmd/wardend/cmd/commands.go
🚧 Files skipped from review as they are similar to previous changes (29)
- tests/framework/snapshot/snapshots.go
- warden/app/ante/cosmos.go
- CHANGELOG.md
- cmd/wardend/config/wardend_config.go
- warden/app/ante/ante.go
- warden/app/ante/evm_handler.go
- warden/x/warden/module/simulation.go
- warden/x/async/types/v1beta1/keys.go
- cmd/wardend/config/config.go
- tests/framework/exec/warden_eth.go
- warden/x/act/types/v1beta1/genesis_test.go
- warden/x/warden/types/v1beta3/genesis_test.go
- warden/app/vemanager/vemanager.go
- precompiles/act/act.go
- cmd/wardend/main.go
- cmd/wardend/config/chain_id.go
- precompiles/async/async.go
- warden/x/sched/types/v1beta1/genesis_test.go
- precompiles/sched/sched.go
- warden/x/sched/module/simulation.go
- warden/x/act/module/simulation.go
- precompiles/warden/warden.go
- warden/app/ante/sched_handler.go
- warden/app/oracle.go
- warden/app/ante/options.go
- warden/x/async/module/simulation.go
- warden/x/sched/keeper/abci.go
- go.mod
- warden/app/upgrades.go
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-06-10T19:20:04.350Z
Learnt from: Pitasi
PR: warden-protocol/wardenprotocol#0
File: :0-0
Timestamp: 2024-06-10T19:20:04.350Z
Learning: Pitasi has implemented error handling in the `Expand` method of `warden/x/warden/keeper/shield.go`, replacing `panic` with error returns.
Applied to files:
cmd/wardend/cmd/root.go
📚 Learning: 2025-02-06T08:48:57.285Z
Learnt from: mn13
PR: warden-protocol/wardenprotocol#1216
File: warden/x/async/keeper/abci.go:212-216
Timestamp: 2025-02-06T08:48:57.285Z
Learning: In the collections package from Cosmos SDK, calling Keys() on an iterator fully consumes and closes the iterator, making an explicit Close() call unnecessary.
Applied to files:
warden/app/export.go
🧬 Code graph analysis (8)
cmd/wardend/config/evm_app_options.go (1)
cmd/wardend/config/config.go (1)
DisplayDenom(24-24)
cmd/wardend/cmd/testnet.go (2)
cmd/wardend/config/config.go (1)
EVMChainID(30-30)cmd/wardend/config/evm_app_options.go (1)
EvmAppOptions(20-41)
prophet/registry.go (1)
prophet/plugins.go (1)
Plugin(12-19)
cmd/wardend/cmd/root.go (5)
warden/app/app.go (2)
NewApp(257-1025)App(184-255)cmd/wardend/config/config.go (2)
EVMChainID(30-30)BaseDenom(26-26)cmd/wardend/config/wardend_config.go (2)
MustGetDefaultNodeHome(34-41)InitAppConfig(122-186)cmd/wardend/config/evm_app_options.go (1)
EvmAppOptions(20-41)cmd/wardend/config/chain_id.go (1)
GetChainIDFromHome(12-29)
warden/app/export.go (1)
warden/app/app.go (1)
App(184-255)
precompiles/precompiles.go (6)
warden/x/async/types/v1beta1/expected_keepers.go (1)
BankKeeper(31-35)warden/x/sched/types/v1beta1/expected_keepers.go (1)
BankKeeper(18-22)precompiles/act/act.go (1)
NewPrecompile(45-70)precompiles/async/async.go (1)
NewPrecompile(45-70)precompiles/sched/sched.go (1)
NewPrecompile(46-73)precompiles/warden/warden.go (1)
NewPrecompile(50-78)
warden/app/app.go (10)
warden/x/act/keeper/keeper.go (2)
Keeper(21-38)NewKeeper(48-91)warden/x/async/keeper/keeper.go (2)
Keeper(34-61)NewKeeper(79-145)warden/x/warden/keeper/keeper.go (2)
Keeper(18-45)NewKeeper(62-122)warden/x/sched/keeper/keeper.go (2)
Keeper(22-34)NewKeeper(44-76)warden/app/oracle.go (1)
SlinkyClient(64-74)cmd/wardend/config/wardend_config.go (2)
GetMaccPerms(98-100)BlockedAddresses(69-95)warden/app/precompiles.go (1)
NewAvailableStaticPrecompiles(82-211)warden/app/genesis.go (4)
GenesisState(19-19)NewMintGenesisState(44-48)NewEVMGenesisState(22-27)NewErc20GenesisState(33-39)warden/app/ante/options.go (1)
HandlerOptions(15-23)warden/app/ante/ante.go (1)
NewAnteHandler(17-58)
warden/app/precompiles.go (4)
precompiles/act/act.go (1)
NewPrecompile(45-70)precompiles/async/async.go (1)
NewPrecompile(45-70)precompiles/sched/sched.go (1)
NewPrecompile(46-73)precompiles/warden/warden.go (1)
NewPrecompile(50-78)
⏰ 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). (8)
- GitHub Check: lint
- GitHub Check: test
- GitHub Check: test
- GitHub Check: lint
- GitHub Check: test
- GitHub Check: lint
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (18)
localnet.just (1)
37-41: Action Required: Generate and Verifygenesis.jsonParametersIt looks like the
genesis.jsonfile wasn’t found at$HOME/.warden/config/genesis.json, so the assertions couldn’t run. Please initialize the localnet to produce the genesis file and then re-verify the key parameters:
- Run the init command to create the genesis:
{{bin}} init localnet --chain-id {{chain_id}} --default-denom award > /dev/null- Ensure the file exists:
ls -l $HOME/.warden/config/genesis.json- Then assert the patched values:
#!/usr/bin/env bash set -euo pipefail GENESIS="$HOME/.warden/config/genesis.json" jq -r '.chain_id' "$GENESIS" jq '.app_state.evm.params.evm_denom' "$GENESIS" jq '.app_state.feemarket.params.no_base_fee' "$GENESIS" jq '.consensus.params.abci.vote_extensions_enable_height' "$GENESIS" # Expect: # - chain_id == "warden_1337-1" # - evm_denom == "award" # - no_base_fee == true # - vote_extensions_enable_height == "2"- Verify that:
chain_idmatches"warden_1337-1".evm_denomis"award".no_base_feeistrue.vote_extensions_enable_heightis"2".Also applies to the snippets at lines 64–70, 76, and 109—please confirm those blocks against the same checks.
warden/app/precompiles.go (7)
128-134: Potential Keeper copy; prefer pointers for server/querier like other modules.Distribution uses value receivers in some SDK versions, but copying Keeper structs can be risky. Align with staking/gov by passing pointers unless the API explicitly requires values.
Option A (if APIs accept pointers):
- distributionkeeper.NewMsgServerImpl(distributionKeeper), - distributionkeeper.NewQuerier(distributionKeeper), + distributionkeeper.NewMsgServerImpl(&distributionKeeper), + distributionkeeper.NewQuerier(&distributionKeeper),If the SDK functions require values, leave as-is. Please confirm against the SDK version in go.mod.
100-104: Options pattern LGTM.Clean, non-breaking configurability for address codecs.
116-123: Staking precompile wiring LGTM.MsgServer, Querier, bankKeeper and codec injection are correct.
165-171: Slashing precompile wiring LGTM.Validator/consensus codecs are configurable; good.
188-196: Passing bankKeeper into Warden precompiles aligns with new balance handler.Good propagation.
3-6: Confirmed Go version is sufficient
- go.mod specifies
go 1.25.0, which is ≥ 1.21 (required for maps.Clone).
No changes are needed.
49-55: It looks like we still need to confirm thatsetupSDKConfig()is invoked before any use ofdefaultOptionals()within your app initialization (i.e., beforePrecompiles(...)is called). Let’s verify the main function’s call order.I’ve requested the first 200 lines of
cmd/wardend/main.goto ensure:
wardendconfig.SetBech32Prefixes(config)andconfig.Seal()are called early.Precompiles(...)(and thusdefaultOptionals()) is invoked only afterward.Once we confirm that ordering, we can mark this as safe. If prefixes aren’t set first, we’ll need to adjust.
precompiles/json/json.go (1)
87-95: Removed stateDB/balance-change hooks: consistent with BankKeeper-based balance handling.Run path stays the same; gas error handling preserved. Looks good.
precompiles/precompiles.go (5)
48-52: act precompile wiring LGTM.
65-70: warden precompile wiring LGTM.
101-106: async precompile wiring LGTM.
121-121: sched precompile wiring LGTM.
37-44: AllNewWardenPrecompilescall sites already include the newbankKeeperargumentI searched for all invocations of
NewWardenPrecompilesand found only one caller inwarden/app/precompiles.go, which already passesbankKeeperas its first parameter:
- warden/app/precompiles.go:188–191
wardenprecompiles, err := wardenprecompiles.NewWardenPrecompiles( bankKeeper, wardenKeeper, actKeeper, … )Since this is the only usage and it’s been updated to match the new signature, no further changes are needed.
warden/app/app.go (1)
375-391: No action needed:erris already declared in this scopeThe short‐variable declaration at line 333 (
prophetP, err := prophet.New()) bringserrinto the function’s scope, so it remains visible at line 382 when callingauthtx.NewTxConfigWithOptions. There is no risk of an undefinederr, and no additional declaration is required.Likely an incorrect or invalid review comment.
prophet/registry.go (2)
10-10: LGTM: explicit plugins map initialization.
Clear and thread-safe with later locking.
30-35: All getPlugin call sites already handle missing pluginsI’ve verified both usages in prophet/plugins.go:
- At lines 30–32 and 66–68, the code checks
if s == niland returns an error.No other call sites were found. No changes are needed.
cmd/wardend/cmd/testnet.go (1)
418-419: Persist full EVM app config using the EVM server config writer.Using cosmos-sdk’s WriteConfigFile may omit EVM/JSON-RPC sections if the template doesn’t include them. Prefer the EVM package writer.
- srvconfig.WriteConfigFile(filepath.Join(nodeDir, "config", "app.toml"), evmCfg) + cosmosevmserverconfig.WriteConfigFile(filepath.Join(nodeDir, "config", "app.toml"), evmCfg)Please confirm the exact function name for your pinned cosmos/evm version (or set the EVM config template before writing if required).
Pitasi
force-pushed
the
remove-depinject
branch
from
12fedf7 to
25776fb
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
warden/x/sched/module/simulation.go (2)
25-28: Fix compile error: remove unused accs in GenerateGenesisState.accs is written but never read; Go will fail to build with “accs declared and not used”.
Apply:
- accs := make([]string, len(simState.Accounts)) - for i, acc := range simState.Accounts { - accs[i] = acc.Address.String() - } + // No account-derived state needed for x/sched genesis.
3-13: Remove leftover scaffolding imports and the “avoid unused import” var block.These pull in unused deps (rand, sdk/types, x/simulation, sched/simulation) and obscure real dependencies.
Apply:
-import ( - "math/rand" - - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/types/module" - simtypes "github.com/cosmos/cosmos-sdk/types/simulation" - "github.com/cosmos/cosmos-sdk/x/simulation" - - schedsimulation "github.com/warden-protocol/wardenprotocol/warden/x/sched/simulation" - types "github.com/warden-protocol/wardenprotocol/warden/x/sched/types/v1beta1" -) +import ( + "github.com/cosmos/cosmos-sdk/types/module" + simtypes "github.com/cosmos/cosmos-sdk/types/simulation" + + types "github.com/warden-protocol/wardenprotocol/warden/x/sched/types/v1beta1" +) @@ -// avoid unused import issue. -var ( - _ = schedsimulation.FindAccount - _ = rand.Rand{} - _ = sdk.AccAddress{} - _ = simulation.MsgEntryKind -) +// (no-op)Also applies to: 15-21
go.mod (1)
3-3: Fix invalidgodirective: use major.minor, not patch.
go.modrequiresgo 1.25, not1.25.0. Patch versions are not allowed and will break tooling.Apply:
-go 1.25.0 +go 1.25
♻️ Duplicate comments (11)
cmd/wardend/config/evm_app_options.go (1)
46-53: Base/display denom registration is reversed (amount scaling bug).Register base denom with factor 1; register display denom with 10^-decimals. This will mis-scale IBC/CLI amounts otherwise.
func setBaseDenom(ci evmtypes.EvmCoinInfo) error { - if err := sdk.RegisterDenom(ci.DisplayDenom, math.LegacyOneDec()); err != nil { + if err := sdk.RegisterDenom(ci.Denom, math.LegacyOneDec()); err != nil { return err } - // sdk.RegisterDenom will automatically overwrite the base denom when the - // new setBaseDenom() are lower than the current base denom's units. - return sdk.RegisterDenom(ci.Denom, math.LegacyNewDecWithPrec(1, int64(ci.Decimals))) + // Register display denom at the correct precision factor. + return sdk.RegisterDenom(ci.DisplayDenom, math.LegacyNewDecWithPrec(1, int64(ci.Decimals))) }justfile (1)
33-35: Checksums likely don’t match resolved wasmvm version; pin or verify.Dynamic version with hard-coded checksums is brittle. Either pin version alongside checksums or add a verify step in CI.
Option A — pin to the verified version and matching checksums:
-wasmvm_version := `go list -m -f '{{ .Version }}' github.com/CosmWasm/wasmvm/v3` -wasmvm_amd64_checksum := 'f2fab51ec2b393ffe6912ff31497c6f8a14c04ad2490a1a3a9fa11a37cb4ce33' -wasmvm_arm64_checksum := 'f82e608707be7b193a78f5a7f7c6163b633a94ca6a0017a7fa3501cc8e9fbff5' +wasmvm_version := 'v3.0.0' +wasmvm_amd64_checksum := 'cefe73f0caa5a9eaba3733c639cdf5040c4746a93c20670ba6c9287fe39448ba' +wasmvm_arm64_checksum := 'a04969b4f931be1d2e2f3f2313a68a20a202693f67559f7aaf1dd97250823aec'Option B — keep dynamic version but add checksum verification in CI (preferred for drift detection).
#!/usr/bin/env bash set -euo pipefail VER="$(go list -m -f '{{ .Version }}' github.com/CosmWasm/wasmvm/v3)" tmp="$(mktemp -d)"; trap 'rm -rf "$tmp"' EXIT curl -fsSL "https://github.com/CosmWasm/wasmvm/releases/download/${VER}/libwasmvm_muslc.x86_64.a" -o "$tmp/a.x86_64.a" curl -fsSL "https://github.com/CosmWasm/wasmvm/releases/download/${VER}/libwasmvm_muslc.aarch64.a" -o "$tmp/a.aarch64.a" a="$(python3 - <<'PY'\nimport hashlib,sys\nprint(hashlib.sha256(open(sys.argv[1],'rb').read()).hexdigest())\nPY "$tmp/a.x86_64.a")" b="$(python3 - <<'PY'\nimport hashlib,sys\nprint(hashlib.sha256(open(sys.argv[1],'rb').read()).hexdigest())\nPY "$tmp/a.aarch64.a")" echo "Resolved: ${VER}"; echo "AMD64 ${a}"; echo "ARM64 ${b}" test "${a}" = {{wasmvm_amd64_checksum}} && test "${b}" = {{wasmvm_arm64_checksum}}cmd/wardend/cmd/testnet.go (6)
227-239: ValidatenumValidatorsearly.Fail fast to avoid resource misuse.
func initTestnetFiles( @@ ) error { + if args.numValidators < 1 || args.numValidators > 100 { + return fmt.Errorf("number of validators must be between 1 and 100, got %d", args.numValidators) + } if args.chainID == "" { args.chainID = "local-4221" }
317-319: Use a dialable loopback in single-machine mode.
0.0.0.0is not dialable by peers; use127.0.0.1.- if args.singleMachine { - ip = "0.0.0.0" + if args.singleMachine { + ip = "127.0.0.1"
534-540: Also persist CometBFTconfig.tomlper node.You mutate
nodeConfigbut never write it; external processes won’t see the changes.// overwrite each validator's genesis file to have a canonical genesis time if err := genutil.ExportGenesisFileWithTime(genFile, chainID, nil, appState, genTime); err != nil { return err } + // persist CometBFT config + if err := cmtconfig.WriteConfigFile(filepath.Join(nodeDir, "config", "config.toml"), nodeConfig); err != nil { + return err + }
481-485: Fix compile error: cannot range over int ininitGenFiles.- for i := range numValidators { + for i := 0; i < numValidators; i++ {
501-507: Fix compile error: cannot range over int incollectGenFiles.- for i := range numValidators { + for i := 0; i < numValidators; i++ {
280-299: Fix compile error: cannot range over int.Replace with a classic for loop.
- for i := range args.numValidators { + for i := 0; i < args.numValidators; i++ {cmd/wardend/cmd/root.go (1)
56-70: Build tempApp with the real EVM chain ID and close it to avoid config mismatches and resource leaksTemp app is created with a constant chain ID and never closed. This can cause EIP‑712/Textual mismatches and lingering sealed globals/resources across CLI invocations. Derive the chain ID from home (fallback to default) and defer Close().
temp := tempDir() // cleanup temp dir after we are done with the tempApp, so we don't leave behind a // new temporary directory for every invocation. See https://github.com/CosmWasm/wasmd/issues/2017 defer os.RemoveAll(temp) - noOpEvmAppOptions := func(_ uint64) error { return nil } - tempApp := app.NewApp( + noOpEvmAppOptions := func(_ uint64) error { return nil } + // try to match CLI/home chainID for correct EIP-712 config + evmForTemp := wardendconfig.EVMChainID + if home := wardendconfig.MustGetDefaultNodeHome(); home != "" { + if cid, err := wardendconfig.GetChainIDFromHome(home); err == nil { + if parsed, err := parseEVMChainID(cid); err == nil { + evmForTemp = parsed + } + } + } + tempApp := app.NewApp( log.NewNopLogger(), dbm.NewMemDB(), nil, true, simtestutil.NewAppOptionsWithFlagHome(temp), - wardendconfig.EVMChainID, + evmForTemp, noOpEvmAppOptions, ) + defer tempApp.Close()warden/app/app.go (2)
914-928: EVMMempool captures nil/old AnteHandler; initialize after SetAnteHandlerMempool is built before ante/post handlers are set, so its AnteHandler is nil/incorrect.
- // mempool initialization (EVM priority nonce mempool) - if evmtypes.GetChainConfig() != nil { - // TODO: Get the actual block gas limit from consensus parameters - mempoolConfig := &evmmempool.EVMMempoolConfig{ - AnteHandler: app.GetAnteHandler(), - BlockGasLimit: 100_000_000, - } - - evmMempool := evmmempool.NewExperimentalEVMMempool(app.CreateQueryContext, logger, app.EVMKeeper, app.FeeMarketKeeper, app.txConfig, app.clientCtx, mempoolConfig) - app.EVMMempool = evmMempool - - app.SetMempool(evmMempool) - checkTxHandler := evmmempool.NewCheckTxHandler(evmMempool) - app.SetCheckTxHandler(checkTxHandler) - } + // (moved below after ante/post handlers)Add right after app.setPostHandler():
// mempool initialization (EVM priority nonce mempool) if evmtypes.GetChainConfig() != nil { // TODO: derive block gas limit from consensus parameters mempoolConfig := &evmmempool.EVMMempoolConfig{ AnteHandler: app.GetAnteHandler(), BlockGasLimit: 100_000_000, } evmMempool := evmmempool.NewExperimentalEVMMempool(app.CreateQueryContext, logger, app.EVMKeeper, app.FeeMarketKeeper, app.txConfig, app.clientCtx, mempoolConfig) app.EVMMempool = evmMempool app.SetMempool(evmMempool) app.SetCheckTxHandler(evmmempool.NewCheckTxHandler(evmMempool)) }Also applies to: 975-983
1048-1060: InitChainer unmarshals AppState into the wrong type; breaks genesisAppStateBytes is the full genesis map, not the EVM-only state. Unmarshal into map[string]json.RawMessage and pass that to InitGenesis.
func (app *App) InitChainer(ctx sdk.Context, req *abci.RequestInitChain) (*abci.ResponseInitChain, error) { - var genesisState cosmosevmtypes.GenesisState + var genesisState map[string]json.RawMessage if err := json.Unmarshal(req.AppStateBytes, &genesisState); err != nil { panic(err) } if err := app.UpgradeKeeper.SetModuleVersionMap(ctx, app.ModuleManager.GetVersionMap()); err != nil { panic(err) } - return app.ModuleManager.InitGenesis(ctx, app.appCodec, genesisState) + return app.ModuleManager.InitGenesis(ctx, app.appCodec, genesisState) }
🧹 Nitpick comments (19)
warden/x/sched/module/simulation.go (2)
41-44: Avoid unnecessary allocation in WeightedOperations.Return nil instead of allocating a zero-length slice.
- operations := make([]simtypes.WeightedOperation, 0) - - return operations + return nil
48-49: Use nil slice for ProposalMsgs.nil is idiomatic and avoids an extra allocation; calling code handles len(nil)==0.
- return []simtypes.WeightedProposalMsg{} + return nilwarden/x/sched/types/v1beta1/genesis_test.go (1)
11-27: Add a negative test case for invalid Params.Increase coverage by asserting
Validate()returns an error for an invalidParamsconfiguration (whateverParams.Validate()deems invalid).If helpful, I can propose a concrete failing case once you confirm the fields
Params.Validate()checks.cmd/wardend/config/chain_id.go (2)
12-29: Harden error handling and empty chain-id detection.Add context to errors, guard empty home, and fail fast when chain-id is unset.
func GetChainIDFromHome(home string) (string, error) { + if home == "" { + return "", fmt.Errorf("home directory must not be empty") + } v := viper.New() - v.AddConfigPath(filepath.Join(home, "config")) + cfgDir := filepath.Join(home, "config") + v.AddConfigPath(cfgDir) v.SetConfigName("client") v.SetConfigType("toml") - if err := v.ReadInConfig(); err != nil { - return "", err + if err := v.ReadInConfig(); err != nil { + return "", fmt.Errorf("read config %s/client.toml: %w", cfgDir, err) } conf := new(config.ClientConfig) - if err := v.Unmarshal(conf); err != nil { - return "", err + if err := v.Unmarshal(conf); err != nil { + return "", fmt.Errorf("unmarshal client config: %w", err) } - return conf.ChainID, nil + if conf.ChainID == "" { + return "", fmt.Errorf("chain-id not set in %s/client.toml", cfgDir) + } + return conf.ChainID, nil }
3-8: Add missing import for formatted errors.Required by the suggested error wrapping.
import ( + "fmt" "path/filepath" "github.com/cosmos/cosmos-sdk/client/config" "github.com/spf13/viper" )prophet/registry.go (2)
8-11: Update comment to reflect plugins terminology.-// registry is a global registry of tasks plugins. +// registry is the global registry of plugins.
18-28: Rename parameter for clarity; consider avoiding panic for duplicates.
- Use a neutral name (plugin) to match type and field.
- Optional: return an error instead of panicking (API change).
-// Register registers a new task plugin with the given unique name. -func Register(name string, task Plugin) { +// Register registers a new plugin with the given unique name. +func Register(name string, plugin Plugin) { @@ - registry.plugins[name] = task + registry.plugins[name] = plugin }go.mod (1)
185-187: Optional: remove redundant indirect Pebble requireThe
replaceon go.mod:10 pins Pebble to v0.0.0-20230928194634-aa077af62593, overriding the indirect require at go.mod:185. Rungo mod tidyto drop the stale require and reduce confusion.precompiles/precompiles.go (1)
3-22: Name collision risk between localcmnandevmcmnimports.Two “common” aliases can confuse maintenance. Consider renaming the local one for clarity.
- cmn "github.com/warden-protocol/wardenprotocol/precompiles/common" + pcommon "github.com/warden-protocol/wardenprotocol/precompiles/common"And replace usages:
- eventsRegistry := cmn.NewEthEventsRegistry() + eventsRegistry := pcommon.NewEthEventsRegistry() - newJsonPrecompile, err := jsonprecompile.NewPrecompile(cmn.NewAbiEncoder()) + newJsonPrecompile, err := jsonprecompile.NewPrecompile(pcommon.NewAbiEncoder())cmd/wardend/config/wardend_config.go (2)
34-41: Prefer error return over panic for default home dir.If this function is used at CLI init, panicking aborts the process. Consider a non-Must variant and handle error at call sites.
-func MustGetDefaultNodeHome() string { +func MustGetDefaultNodeHome() string { // keep for BC ... } + +func GetDefaultNodeHome() (string, error) { + return clienthelpers.GetNodeHomeDirectory(".warden") +}
69-95: Pre-allocate blocked address map and avoid intermediate slice.Minor perf/readability cleanups.
-func BlockedAddresses() map[string]bool { - blockedAddrs := make(map[string]bool) - - maccPerms := GetMaccPerms() - - accs := make([]string, 0, len(maccPerms)) - for acc := range maccPerms { - accs = append(accs, acc) - } - sort.Strings(accs) - - for _, acc := range accs { - blockedAddrs[authtypes.NewModuleAddress(acc).String()] = true - } +func BlockedAddresses() map[string]bool { + maccPerms := GetMaccPerms() + blockedAddrs := make(map[string]bool, len(maccPerms)+len(corevm.PrecompiledAddressesBerlin)+len(evmtypes.AvailableStaticPrecompiles)) + + // determinism: stable order for reproducible maps if needed elsewhere + accs := make([]string, 0, len(maccPerms)) + for acc := range maccPerms { + accs = append(accs, acc) + } + sort.Strings(accs) + for _, acc := range accs { + blockedAddrs[authtypes.NewModuleAddress(acc).String()] = true + }warden/app/precompiles.go (1)
154-161: Rename paramcodecto avoid shadowing the imported package.Improves clarity.
- govPrecompile, err := govprecompile.NewPrecompile( + govPrecompile, err := govprecompile.NewPrecompile( govkeeper.NewMsgServerImpl(&govKeeper), govkeeper.NewQueryServer(&govKeeper), bankKeeper, - codec, + codec, // keep callsite options.AddressCodec, )Outside selected range (function signature), change:
- codec codec.Codec, + cdc codec.Codec,and the call:
- govkeeper.NewQueryServer(&govKeeper), - bankKeeper, - codec, + govkeeper.NewQueryServer(&govKeeper), + bankKeeper, + cdc,cmd/wardend/cmd/testnet.go (3)
250-257: Enable EVM JSON-RPC explicitly for testnet.Defaults may be disabled; ensure it’s on for local testing.
evmCfg := evmdconfig.EVMAppConfig{ Config: *appConfig, EVM: *evm, - JSONRPC: *cosmosevmserverconfig.DefaultJSONRPCConfig(), + JSONRPC: *func() *cosmosevmserverconfig.JSONRPCConfig { c := cosmosevmserverconfig.DefaultJSONRPCConfig(); c.Enable = true; return c }(), TLS: *cosmosevmserverconfig.DefaultTLSConfig(), }
621-629: Avoid Scanln pitfalls; just wait for newline robustly.
fmt.Scanln()can error on empty input. Use a buffered read.- if _, err := fmt.Scanln(); err != nil { // wait for Enter Key - return err - } + _, _ = bufio.NewReader(os.Stdin).ReadBytes('\n')
634-641: Remove unused temp dir.Temp dir is created and removed but never used.
- dir, err := os.MkdirTemp("", "evm") - if err != nil { - panic(fmt.Sprintf("failed creating temporary directory: %v", err)) - } - defer os.RemoveAll(dir) + // no temp dir needed for in-memory DBcmd/wardend/cmd/root.go (1)
95-95: Avoid shadowing initClientCtxUsing := redefines initClientCtx in this scope; prefer assignment to keep a single variable and reduce confusion.
- initClientCtx, err := client.ReadPersistentCommandFlags(initClientCtx, cmd.Flags()) + initClientCtx, err = client.ReadPersistentCommandFlags(initClientCtx, cmd.Flags())warden/app/app.go (3)
1428-1434: Shadowing net/url import with local variable ‘url’Local variable named url shadows the imported package, harms readability.
- url, err := url.Parse(u) + parsedURL, err := url.Parse(u) if err != nil { panic(fmt.Errorf("invalid pricepred url: %s", u)) } - prophet.Register("pricepred", pricepred.NewPricePredictorSolidity(url)) + prophet.Register("pricepred", pricepred.NewPricePredictorSolidity(parsedURL))
332-341: Prophet sidecar is started from NewApp; ensure tempApp never starts itNewApp may start the sidecar if rpc.laddr is set in appOpts. The CLI’s tempApp should never start external services. Verify temp AppOptions never include rpc.laddr; otherwise guard this block behind loadLatest or a dedicated option.
Would you like a follow-up diff to gate prophet startup behind a specific appOpt (e.g., prophet.enabled)?
1302-1318: Close gracefully aggregates errors; consider closing prophet tooClose() does not stop the prophet sidecar. If prophet exposes a stopper, call it here.
I can wire a best-effort shutdown (ignore if not present).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (64)
go.sumis excluded by!**/*.sumtests/testdata/snapshot-base/config/app.toml.tmplis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/genesis.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/gentx/gentx-38d79e1aae59e234ad6a60fd92896082bf41c64e.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/gentx/gentx-6945f3e30b02b9e34673acc22a18c3688ed57a19.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/node_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/priv_validator_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/keyring-test/80bd83f2b3f9f098fe4cd54891cdc86595a6d479.addressis excluded by!tests/testdata/**tests/testdata/snapshot-base/keyring-test/alice.infois excluded by!tests/testdata/**tests/testdata/snapshot-base/keyring-test/c8af84b4401237ec5048090f46c6327332be71cf.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/app.toml.tmplis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/genesis.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/gentx/gentx-32688592d17c81e774a966370c9366b35845614e.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/gentx/gentx-c1add3510774b379cfe92d361f1aa521dc9c9fc6.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/node_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/priv_validator_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/2ff01d371641069bd86371069b21a11f69383a33.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/32a224cecfe0a0660c93073fbcb6afebdbffa5ab.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/461cb598a46fcc43bef9044774afd0c4d66b54c9.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/63675504137926347258e5687f45cbf2b1f87b56.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/7daa0db7bba477374f17bf0cc2fa96d344bb8258.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/bob.infois excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/f114af4db343f425c8633979922e6f668f47dbc1.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/val.infois excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/writer.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/app.toml.tmplis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/genesis.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/gentx/gentx-2093ee4e76ca7a6460e5b92915bcbe61c8137495.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/gentx/gentx-a587c87739afa2c555eadf8b225cf7f389e31fa1.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/node_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/priv_validator_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/056d037ac7ae938ae179115f64a160d6976fc501.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/0fbc33e8e29268d962ebf0d9db19bf2715aee1d1.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/1856bab8f08b38ece497419342ec2ff2ba79b884.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/356e83fe984f8cff0e34868a9e239f7623d77644.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/460023f64738ff322a330aaadabe989057e72a66.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/640d841942f70598dea2ecc203be2efe0754fe37.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/6b8720d45650a5f4718bb85c0b3c713be7dabf83.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/alice.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/b730c0872fcb7106b5ab7fbaca574c4582e2a287.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/bddff3c30fabae236823a53b91505b5f445f973e.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/befc8528482269aef5c988845d0e365e1de01458.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/bf4bc4afeee16236f73202e7e93aafdab879c304.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/bob.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/charlie.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/dave.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/eb45ceb51c38527501b095031fa54c1b2911f981.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/erin.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/frank.infois excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/app.toml.tmplis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/genesis.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/gentx/gentx-2601681988711cb256c1068a6ae0fe35755fdb26.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/gentx/gentx-dbafff433fe3c3711b6ade63130895b84e777eb3.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/node_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/priv_validator_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/16159a91f2b78293bf6e1d0525433a6ad9fa7807.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/3005241c8fb90192342c9d58b86b434f8ba1d807.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/4596f61417e0c4fae43c201a9a0cd06e3b403ffd.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/79fce8f9a2fd6d67aba6a83f8d2b499c6b9a4eb1.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/alice.infois excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/b26f6dc33e0a93a55c3caa3dbf6e58155d6ca55f.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/bob.infois excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/dave.infois excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/e0ace9b0c67b860bd22cffcbb08591f3cc3b6e42.addressis excluded by!tests/testdata/**
📒 Files selected for processing (73)
CHANGELOG.md(1 hunks)cmd/wardend/cmd/commands.go(0 hunks)cmd/wardend/cmd/config.go(0 hunks)cmd/wardend/cmd/root.go(1 hunks)cmd/wardend/cmd/testnet.go(1 hunks)cmd/wardend/cmd/wait.go(0 hunks)cmd/wardend/config/chain_id.go(1 hunks)cmd/wardend/config/config.go(2 hunks)cmd/wardend/config/evm_app_options.go(1 hunks)cmd/wardend/config/wardend_config.go(1 hunks)cmd/wardend/main.go(1 hunks)go.mod(9 hunks)justfile(1 hunks)localnet.just(3 hunks)precompiles/act/act.go(2 hunks)precompiles/async/async.go(2 hunks)precompiles/json/json.go(1 hunks)precompiles/precompiles.go(6 hunks)precompiles/sched/sched.go(2 hunks)precompiles/slinky/slinky.go(0 hunks)precompiles/warden/warden.go(6 hunks)prophet/registry.go(1 hunks)proto/warden/act/v1beta1/query.proto(0 hunks)tests/framework/exec/warden_eth.go(3 hunks)tests/framework/snapshot/snapshots.go(1 hunks)warden/app/ante.go(0 hunks)warden/app/ante/ante.go(1 hunks)warden/app/ante/cosmos.go(1 hunks)warden/app/ante/evm_handler.go(1 hunks)warden/app/ante/options.go(1 hunks)warden/app/ante/sched_handler.go(1 hunks)warden/app/app.go(4 hunks)warden/app/app_config.go(0 hunks)warden/app/config.go(0 hunks)warden/app/export.go(7 hunks)warden/app/genesis.go(0 hunks)warden/app/legacy.go(0 hunks)warden/app/oracle.go(4 hunks)warden/app/precompiles.go(6 hunks)warden/app/sim_bench_test.go(0 hunks)warden/app/sim_test.go(0 hunks)warden/app/upgrades.go(1 hunks)warden/app/vemanager/vemanager.go(2 hunks)warden/testutil/network/network.go(0 hunks)warden/testutil/nullify/nullify.go(0 hunks)warden/testutil/sample/sample.go(0 hunks)warden/x/act/module/autocli.go(0 hunks)warden/x/act/module/genesis.go(0 hunks)warden/x/act/module/genesis_test.go(0 hunks)warden/x/act/module/simulation.go(1 hunks)warden/x/act/types/v1beta1/codec.go(0 hunks)warden/x/act/types/v1beta1/genesis.go(0 hunks)warden/x/act/types/v1beta1/genesis_test.go(1 hunks)warden/x/async/module/genesis.go(0 hunks)warden/x/async/module/genesis_test.go(0 hunks)warden/x/async/module/simulation.go(1 hunks)warden/x/async/types/v1beta1/genesis.go(0 hunks)warden/x/async/types/v1beta1/keys.go(1 hunks)warden/x/sched/keeper/abci.go(3 hunks)warden/x/sched/module/autocli.go(0 hunks)warden/x/sched/module/genesis.go(0 hunks)warden/x/sched/module/genesis_test.go(0 hunks)warden/x/sched/module/simulation.go(1 hunks)warden/x/sched/types/v1beta1/codec.go(0 hunks)warden/x/sched/types/v1beta1/genesis.go(0 hunks)warden/x/sched/types/v1beta1/genesis_test.go(1 hunks)warden/x/warden/module/autocli.go(0 hunks)warden/x/warden/module/genesis.go(0 hunks)warden/x/warden/module/genesis_test.go(0 hunks)warden/x/warden/module/simulation.go(1 hunks)warden/x/warden/types/v1beta3/codec.go(0 hunks)warden/x/warden/types/v1beta3/genesis.go(0 hunks)warden/x/warden/types/v1beta3/genesis_test.go(1 hunks)
💤 Files with no reviewable changes (33)
- warden/x/act/module/genesis_test.go
- warden/x/act/module/autocli.go
- warden/x/sched/types/v1beta1/codec.go
- warden/x/async/module/genesis.go
- warden/x/sched/module/autocli.go
- warden/x/warden/types/v1beta3/genesis.go
- warden/x/warden/module/genesis_test.go
- warden/x/warden/module/autocli.go
- warden/testutil/nullify/nullify.go
- cmd/wardend/cmd/config.go
- warden/x/async/module/genesis_test.go
- warden/app/genesis.go
- warden/x/async/types/v1beta1/genesis.go
- precompiles/slinky/slinky.go
- warden/app/config.go
- warden/app/sim_bench_test.go
- warden/x/act/types/v1beta1/genesis.go
- proto/warden/act/v1beta1/query.proto
- warden/x/act/module/genesis.go
- warden/testutil/sample/sample.go
- warden/x/warden/module/genesis.go
- warden/x/sched/module/genesis.go
- warden/x/warden/types/v1beta3/codec.go
- warden/x/sched/types/v1beta1/genesis.go
- warden/x/act/types/v1beta1/codec.go
- warden/x/sched/module/genesis_test.go
- warden/app/app_config.go
- warden/app/ante.go
- warden/app/legacy.go
- warden/app/sim_test.go
- warden/testutil/network/network.go
- cmd/wardend/cmd/commands.go
- cmd/wardend/cmd/wait.go
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (25)
- warden/app/ante/evm_handler.go
- warden/x/act/types/v1beta1/genesis_test.go
- warden/app/ante/sched_handler.go
- cmd/wardend/main.go
- warden/x/warden/module/simulation.go
- warden/app/vemanager/vemanager.go
- precompiles/act/act.go
- warden/app/ante/options.go
- cmd/wardend/config/config.go
- warden/app/ante/ante.go
- warden/x/sched/keeper/abci.go
- warden/x/act/module/simulation.go
- tests/framework/exec/warden_eth.go
- precompiles/sched/sched.go
- precompiles/warden/warden.go
- tests/framework/snapshot/snapshots.go
- warden/x/async/module/simulation.go
- precompiles/json/json.go
- localnet.just
- warden/x/warden/types/v1beta3/genesis_test.go
- warden/x/async/types/v1beta1/keys.go
- warden/app/export.go
- warden/app/oracle.go
- warden/app/upgrades.go
- warden/app/ante/cosmos.go
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-24T13:01:55.558Z
Learnt from: backsapc
PR: warden-protocol/wardenprotocol#810
File: cmd/wardenkms/wardenkms.go:45-50
Timestamp: 2024-10-24T13:01:55.558Z
Learning: In the `cmd/wardenkms/wardenkms.go` file and the codebase, use `Grpc` instead of `GRPC` for acronym casing in type names, such as `GrpcNodeConfig` and `GrpcNodeConfigDecoder`, even though Go naming conventions suggest capitalizing acronyms.
Applied to files:
cmd/wardend/config/evm_app_options.go
📚 Learning: 2024-06-10T19:20:04.350Z
Learnt from: Pitasi
PR: warden-protocol/wardenprotocol#0
File: :0-0
Timestamp: 2024-06-10T19:20:04.350Z
Learning: Pitasi has implemented error handling in the `Expand` method of `warden/x/warden/keeper/shield.go`, replacing `panic` with error returns.
Applied to files:
cmd/wardend/cmd/root.go
🧬 Code graph analysis (1)
warden/x/sched/types/v1beta1/genesis_test.go (1)
warden/x/sched/types/v1beta1/genesis.pb.go (3)
GenesisState(28-31)GenesisState(35-35)GenesisState(36-38)
⏰ 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). (7)
- GitHub Check: lint
- GitHub Check: lint
- GitHub Check: test
- GitHub Check: lint
- GitHub Check: test
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (10)
warden/x/sched/types/v1beta1/genesis_test.go (1)
24-26: No changes needed:DefaultParams()returns the zero‐valueParamsand there is noValidate()logic onGenesisStateorParams, so&GenesisState{}is functionally equivalent to&GenesisState{Params: types.DefaultParams()}.precompiles/async/async.go (1)
45-49: Add nil-check for bankKeeper in NewPrecompile
Reject a nil BankKeeper up front to prevent a runtime nil-pointer in Run. All existing callers pass a non-nil value.justfile (1)
27-29: Good: strict shell mode.set -euxo pipefail makes the deploy flow fail-fast and easier to debug.
precompiles/precompiles.go (1)
36-44: BankKeeper wiring across precompiles looks good.Constructor signatures and event registry integration are consistent with the evm precompile common.BankKeeper pattern.
Also applies to: 48-53, 65-71, 101-106, 121-129
cmd/wardend/config/wardend_config.go (1)
140-152: LGTM: AppConfig/EVM/JSON-RPC/Oracle defaults are coherent.Enabling JSON-RPC by default and wiring EVM chain ID via parameter is consistent with the rest of the PR.
Also applies to: 160-186
warden/app/precompiles.go (4)
188-196: LGTM: Warden precompiles wiring via BankKeeper and opts-based codecs.Clean separation with options and coherent constructor usage.
127-137: Awaiting script results to confirm parameter types and receiver expectations.
165-172: Verify slashing MsgServerImpl argument
Confirm the signature ofslashingkeeper.NewMsgServerImpland the declared type ofslashingKeeper; only switch to&slashingKeeperif the method requires a pointer to the keeper.
49-56: No action required—Bech32 prefixes are set before precompile init
setupSDKConfig()incmd/wardend/main.gosets and seals all Bech32 prefixes beforeNewAvailableStaticPrecompilesruns, and tests initialize prefixes in theirinitfunctions.warden/app/app.go (1)
374-386: err reuse is now valid; keep it that wayUsing the function-scoped err declared earlier fixes the previous compile error.
Pitasi
force-pushed
the
remove-depinject
branch
from
25776fb to
ae80e60
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
localnet.just (1)
101-103: Bug: wrong variable when deploying osmosis analyzerDeploys ethereum_analyzer instead of osmosis_analyzer.
- just deploy-contract $ethereum_analyzer_wasm + just deploy-contract $osmosis_analyzer_wasm
♻️ Duplicate comments (13)
go.mod (2)
143-143: Remove depinject to align with PR objective.The PR removes depinject, but go.mod still lists cosmossdk.io/depinject (indirect). Drop it after ensuring no imports remain.
- cosmossdk.io/depinject v1.2.1 // indirect#!/usr/bin/env bash # Ensure no code still imports depinject rg -nP '\bcosmossdk\.io/depinject\b' -S -g '!**/vendor/**' -C2
380-381: Drop non-canonical YAML module path (use gopkg.in/yaml.v2).go.yaml.in/yaml/v2 is redundant and non-canonical; gopkg.in/yaml.v2 is already present.
- go.yaml.in/yaml/v2 v2.4.2 // indirect#!/usr/bin/env bash # Confirm no references to the non-canonical path remain rg -nP 'go\.yaml\.in/yaml/v2' -S -g '!**/vendor/**' -C2cmd/wardend/cmd/testnet.go (8)
317-319: Use loopback (127.0.0.1) in single-machine mode, not 0.0.0.0.0.0.0.0 isn’t dialable for peers; this breaks persistent peer memos.
- if args.singleMachine { - ip = "0.0.0.0" + if args.singleMachine { + ip = "127.0.0.1"
418-419: Handle app.toml write errors.Don’t ignore WriteConfigFile errors.
- srvconfig.WriteConfigFile(filepath.Join(nodeDir, "config", "app.toml"), evmCfg) + if err := srvconfig.WriteConfigFile(filepath.Join(nodeDir, "config", "app.toml"), evmCfg); err != nil { + return err + }
536-540: Persist CometBFT config (config.toml) per node.After exporting genesis, write the (mutated) Comet config to disk.
if err := genutil.ExportGenesisFileWithTime(genFile, chainID, nil, appState, genTime); err != nil { return err } + if err := cmtconfig.WriteConfigFile( + filepath.Join(nodeDir, "config", "config.toml"), + nodeConfig, + ); err != nil { + return err + }
236-243: Validate validator count early to prevent resource misuse.Fail fast on invalid counts.
if args.chainID == "" { args.chainID = "local-4221" } +if args.numValidators < 1 || args.numValidators > 100 { + return fmt.Errorf("number of validators must be between 1 and 100, got %d", args.numValidators) +}
280-289: Compile error: cannot range over int.Replace the invalid range with an indexed for-loop.
- for i := range args.numValidators { + for i := 0; i < args.numValidators; i++ { var ( portOffset int evmPortOffset int ) if args.singleMachine { portOffset = i evmPortOffset = i * 10
481-485: Compile error: cannot range over int in initGenFiles.Same fix applies.
- for i := range numValidators { + for i := 0; i < numValidators; i++ { if err := genDoc.SaveAs(genFiles[i]); err != nil { return err } }
501-507: Compile error: cannot range over int in collectGenFiles.Fix the loop construct.
- for i := range numValidators { + for i := 0; i < numValidators; i++ { if singleMachine { portOffset := i nodeConfig.RPC.ListenAddress = fmt.Sprintf("tcp://0.0.0.0:%d", rpcPortStart+portOffset) nodeConfig.P2P.ListenAddress = fmt.Sprintf("tcp://0.0.0.0:%d", p2pPortStart+portOffset) }
564-568: Compile error and IP math bug: cannot range over int; potential /24 overflow.Implement safe arithmetic and bound to the same /24.
- for range i { - ipv4[3]++ - } + orig := (uint32(ipv4[0]) << 24) | (uint32(ipv4[1]) << 16) | (uint32(ipv4[2]) << 8) | uint32(ipv4[3]) + off := int64(i) + if off < 0 && uint32(-off) > orig { + return "", fmt.Errorf("IP address underflow: %s + (%d)", ip, i) + } + newIP := uint32(int64(orig) + off) + // stay within the same /24 subnet + if (newIP>>8) != (orig>>8) { + return "", fmt.Errorf("IP address overflow into another subnet: %s + %d", ip, i) + } + ipv4[0] = byte(newIP >> 24) + ipv4[1] = byte(newIP >> 16) + ipv4[2] = byte(newIP >> 8) + ipv4[3] = byte(newIP)warden/app/app.go (2)
1048-1060: InitChainer unmarshals wrong type; breaks genesis initialization.Unmarshal AppState into map[string]json.RawMessage and pass that to ModuleManager.InitGenesis.
- var genesisState cosmosevmtypes.GenesisState + var genesisState map[string]json.RawMessage if err := json.Unmarshal(req.AppStateBytes, &genesisState); err != nil { panic(err) } - return app.ModuleManager.InitGenesis(ctx, app.appCodec, genesisState) + return app.ModuleManager.InitGenesis(ctx, app.appCodec, genesisState)
914-929: Move EVM mempool initialization to after app.SetAnteHandler
Mempool setup at lines 914–929 runs beforeapp.SetAnteHandler(...)at line 1374, soapp.GetAnteHandler()returns nil.- // mempool initialization (EVM priority nonce mempool) - if evmtypes.GetChainConfig() != nil { - mempoolConfig := &evmmempool.EVMMempoolConfig{ - AnteHandler: app.GetAnteHandler(), - BlockGasLimit: 100_000_000, - } - evmMempool := evmmempool.NewExperimentalEVMMempool(app.CreateQueryContext, logger, app.EVMKeeper, app.FeeMarketKeeper, app.txConfig, app.clientCtx, mempoolConfig) - app.EVMMempool = evmMempool - app.SetMempool(evmMempool) - app.SetCheckTxHandler(evmmempool.NewCheckTxHandler(evmMempool)) - } + // move this block to immediately after app.SetAnteHandler(...) at line 1374cmd/wardend/cmd/root.go (1)
61-70: Temp app uses constant EVM chain ID — risk of EIP‑712 mismatch and global configurator conflicts.Resolve evmChainID from home (or client config) before creating tempApp.
noOpEvmAppOptions := func(_ uint64) error { return nil } + // derive EVM chain ID for temp app (avoid EIP-712/config mismatches) + evmForTemp := wardendconfig.EVMChainID + if home := wardendconfig.MustGetDefaultNodeHome(); home != "" { + if cid, err := wardendconfig.GetChainIDFromHome(home); err == nil { + if parsed, err := parseEVMChainID(cid); err == nil { + evmForTemp = parsed + } + } + } tempApp := app.NewApp( log.NewNopLogger(), dbm.NewMemDB(), nil, true, simtestutil.NewAppOptionsWithFlagHome(temp), - wardendconfig.EVMChainID, + evmForTemp, noOpEvmAppOptions, )
🧹 Nitpick comments (13)
prophet/registry.go (4)
8-8: Fix outdated comment (“tasks plugins”)Comment still references “tasks”. Update for accuracy.
-// registry is a global registry of tasks plugins. +// registry is a global registry of plugins.
9-16: Rename internal typerfor claritySingle-letter type obscures intent. Rename to
pluginRegistryand keep the package-level var asregistry.-var registry = r{ +var registry = pluginRegistry{ plugins: make(map[string]Plugin), } -type r struct { +type pluginRegistry struct { rw sync.RWMutex plugins map[string]Plugin }
3-6: Deterministic order for RegisteredPluginsMap iteration order is random; sort the names to avoid flakiness and improve UX. Add the import.
import ( "fmt" + "sort" "sync" )- plugins := make([]string, 0, len(registry.plugins)) - for name := range registry.plugins { - plugins = append(plugins, name) - } + plugins := make([]string, 0, len(registry.plugins)) + for name := range registry.plugins { + plugins = append(plugins, name) + } + sort.Strings(plugins) return pluginsAlso applies to: 41-46
9-16: Consider test isolation (global state)A package-global registry can leak state across tests. Consider adding a constructor (e.g., NewRegistry for non-global instances) or a test-only Reset() to clear
registry.plugins.localnet.just (3)
37-37: Pass --home to init to avoid relying on default $HOMEMakes the script resilient if WARDEN_HOME ever changes or tests run under a different home.
- {{bin}} init localnet --chain-id {{chain_id}} --default-denom award > /dev/null + {{bin}} init localnet --home "$WARDEN_HOME" --chain-id {{chain_id}} --default-denom award > /dev/nullTip: consider doing the same for subsequent config/keys/genesis commands for consistency.
109-109: Pass --home to start for consistency with WARDEN_HOMEPrevents accidental use of a different home in CI/local runs.
- (post_start &) && {{bin}} start + (post_start &) && {{bin}} start --home "$WARDEN_HOME"
4-5: Remove or use unused val2/val2_mnemonicCurrently defined but unused; either add val2 as a second validator/account or drop these lines.
- val2 := "warden194j3dx4t7u80765k2puu3082vc39yfqnvrxqu7" - val2_mnemonic := "door blast other element embark noodle maple agent drastic bunker swarm logic wide deputy exhibit bicycle evil exile air other mesh foot tackle spot" + # (optional) define/use a second validator/account, or omit these variableswarden/x/sched/keeper/abci.go (1)
134-145: Future-proof EVM signer selection.Using NewLondonSigner with a LegacyTx works, but LatestSignerForChainID is safer across hard forks.
- signedEthTx, err := ethtypes.SignNewTx( - privkey, - ethtypes.NewLondonSigner(new(big.Int).SetUint64(chainID)), + signedEthTx, err := ethtypes.SignNewTx( + privkey, + ethtypes.LatestSignerForChainID(new(big.Int).SetUint64(chainID)), ðtypes.LegacyTx{warden/app/oracle.go (1)
259-265: Recover + re-panic is fine; consider logging via app logger for consistency.debug.PrintStack() goes to stderr. Logging the panic with app.Logger() improves observability.
- defer func() { - if err := recover(); err != nil { - debug.PrintStack() - panic(err) - } - }() + defer func() { + if r := recover(); r != nil { + debug.PrintStack() + ctx.Logger().Error("panic in PreBlocker", "panic", r) + panic(r) + } + }()cmd/wardend/cmd/testnet.go (1)
607-610: Typo in error message (“testnests”).Fix for clarity.
- "testnests directory already exists for chain-id '%s': %s, please remove or select a new --chain-id", + "testnets directory already exists for chain-id '%s': %s, please remove or select a new --chain-id",precompiles/warden/warden.go (2)
74-76: Minor consistency: use p.Address() when setting the address.Other precompiles use p.SetAddress(p.Address()). Align for uniformity.
- p.SetAddress(common.HexToAddress(PrecompileAddress)) + p.SetAddress(p.Address())
200-235: Avoid panic on unknown method in IsTransaction.A panic here can crash the node if reached unexpectedly. Prefer a safe default.
func (*Precompile) IsTransaction(method *abi.Method) bool { switch method.Name { // cases ... - } - - panic(fmt.Errorf("warden precompile: method not exists: %s", method)) + default: + // Unknown method: treat as a query; Run/ABI decode will error out safely. + return false + } }cmd/wardend/cmd/root.go (1)
153-155: Don’t ignore error from ReadFromClientConfig.Guard or log; PreRunE already does strict handling.
- initClientCtx, _ = clientcfg.ReadFromClientConfig(initClientCtx) - autoCliOpts.ClientCtx = initClientCtx + if cctx, err := clientcfg.ReadFromClientConfig(initClientCtx); err == nil { + initClientCtx = cctx + } + autoCliOpts.ClientCtx = initClientCtx
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (65)
go.sumis excluded by!**/*.sumtests/slinky/go.sumis excluded by!**/*.sumtests/testdata/snapshot-base/config/app.toml.tmplis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/genesis.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/gentx/gentx-38d79e1aae59e234ad6a60fd92896082bf41c64e.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/gentx/gentx-6945f3e30b02b9e34673acc22a18c3688ed57a19.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/node_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/priv_validator_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/keyring-test/80bd83f2b3f9f098fe4cd54891cdc86595a6d479.addressis excluded by!tests/testdata/**tests/testdata/snapshot-base/keyring-test/alice.infois excluded by!tests/testdata/**tests/testdata/snapshot-base/keyring-test/c8af84b4401237ec5048090f46c6327332be71cf.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/app.toml.tmplis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/genesis.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/gentx/gentx-32688592d17c81e774a966370c9366b35845614e.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/gentx/gentx-c1add3510774b379cfe92d361f1aa521dc9c9fc6.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/node_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/priv_validator_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/2ff01d371641069bd86371069b21a11f69383a33.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/32a224cecfe0a0660c93073fbcb6afebdbffa5ab.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/461cb598a46fcc43bef9044774afd0c4d66b54c9.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/63675504137926347258e5687f45cbf2b1f87b56.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/7daa0db7bba477374f17bf0cc2fa96d344bb8258.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/bob.infois excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/f114af4db343f425c8633979922e6f668f47dbc1.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/val.infois excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/writer.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/app.toml.tmplis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/genesis.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/gentx/gentx-2093ee4e76ca7a6460e5b92915bcbe61c8137495.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/gentx/gentx-a587c87739afa2c555eadf8b225cf7f389e31fa1.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/node_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/priv_validator_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/056d037ac7ae938ae179115f64a160d6976fc501.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/0fbc33e8e29268d962ebf0d9db19bf2715aee1d1.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/1856bab8f08b38ece497419342ec2ff2ba79b884.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/356e83fe984f8cff0e34868a9e239f7623d77644.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/460023f64738ff322a330aaadabe989057e72a66.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/640d841942f70598dea2ecc203be2efe0754fe37.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/6b8720d45650a5f4718bb85c0b3c713be7dabf83.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/alice.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/b730c0872fcb7106b5ab7fbaca574c4582e2a287.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/bddff3c30fabae236823a53b91505b5f445f973e.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/befc8528482269aef5c988845d0e365e1de01458.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/bf4bc4afeee16236f73202e7e93aafdab879c304.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/bob.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/charlie.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/dave.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/eb45ceb51c38527501b095031fa54c1b2911f981.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/erin.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/frank.infois excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/app.toml.tmplis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/genesis.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/gentx/gentx-2601681988711cb256c1068a6ae0fe35755fdb26.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/gentx/gentx-dbafff433fe3c3711b6ade63130895b84e777eb3.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/node_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/priv_validator_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/16159a91f2b78293bf6e1d0525433a6ad9fa7807.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/3005241c8fb90192342c9d58b86b434f8ba1d807.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/4596f61417e0c4fae43c201a9a0cd06e3b403ffd.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/79fce8f9a2fd6d67aba6a83f8d2b499c6b9a4eb1.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/alice.infois excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/b26f6dc33e0a93a55c3caa3dbf6e58155d6ca55f.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/bob.infois excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/dave.infois excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/e0ace9b0c67b860bd22cffcbb08591f3cc3b6e42.addressis excluded by!tests/testdata/**
📒 Files selected for processing (79)
CHANGELOG.md(1 hunks)cmd/wardend/cmd/commands.go(0 hunks)cmd/wardend/cmd/config.go(0 hunks)cmd/wardend/cmd/root.go(1 hunks)cmd/wardend/cmd/testnet.go(1 hunks)cmd/wardend/cmd/wait.go(0 hunks)cmd/wardend/config/chain_id.go(1 hunks)cmd/wardend/config/config.go(2 hunks)cmd/wardend/config/evm_app_options.go(1 hunks)cmd/wardend/config/wardend_config.go(1 hunks)cmd/wardend/main.go(1 hunks)go.mod(9 hunks)justfile(1 hunks)localnet.just(3 hunks)precompiles/act/act.go(2 hunks)precompiles/async/async.go(2 hunks)precompiles/json/json.go(1 hunks)precompiles/precompiles.go(6 hunks)precompiles/sched/sched.go(2 hunks)precompiles/slinky/slinky.go(0 hunks)precompiles/warden/warden.go(6 hunks)prophet/registry.go(1 hunks)proto/warden/act/v1beta1/query.proto(0 hunks)tests/framework/exec/warden_eth.go(3 hunks)tests/framework/snapshot/snapshots.go(1 hunks)tests/slinky/go.mod(10 hunks)tests/slinky/slinky_test.go(2 hunks)warden/app/ante.go(0 hunks)warden/app/ante/ante.go(1 hunks)warden/app/ante/cosmos.go(1 hunks)warden/app/ante/evm_handler.go(1 hunks)warden/app/ante/options.go(1 hunks)warden/app/ante/sched_handler.go(1 hunks)warden/app/app.go(4 hunks)warden/app/app_config.go(0 hunks)warden/app/config.go(0 hunks)warden/app/export.go(7 hunks)warden/app/genesis.go(0 hunks)warden/app/legacy.go(0 hunks)warden/app/oracle.go(4 hunks)warden/app/precompiles.go(6 hunks)warden/app/sim_bench_test.go(0 hunks)warden/app/sim_test.go(0 hunks)warden/app/upgrades.go(1 hunks)warden/app/vemanager/vemanager.go(2 hunks)warden/testutil/network/network.go(0 hunks)warden/testutil/nullify/nullify.go(0 hunks)warden/testutil/sample/sample.go(0 hunks)warden/x/act/module/autocli.go(0 hunks)warden/x/act/module/genesis.go(0 hunks)warden/x/act/module/genesis_test.go(0 hunks)warden/x/act/module/module.go(0 hunks)warden/x/act/module/simulation.go(1 hunks)warden/x/act/types/v1beta1/codec.go(0 hunks)warden/x/act/types/v1beta1/genesis.go(0 hunks)warden/x/act/types/v1beta1/genesis_test.go(1 hunks)warden/x/async/module/genesis.go(0 hunks)warden/x/async/module/genesis_test.go(0 hunks)warden/x/async/module/module.go(0 hunks)warden/x/async/module/simulation.go(1 hunks)warden/x/async/types/v1beta1/genesis.go(0 hunks)warden/x/async/types/v1beta1/keys.go(1 hunks)warden/x/sched/keeper/abci.go(3 hunks)warden/x/sched/module/autocli.go(0 hunks)warden/x/sched/module/genesis.go(0 hunks)warden/x/sched/module/genesis_test.go(0 hunks)warden/x/sched/module/module.go(0 hunks)warden/x/sched/module/simulation.go(1 hunks)warden/x/sched/types/v1beta1/codec.go(0 hunks)warden/x/sched/types/v1beta1/genesis.go(0 hunks)warden/x/sched/types/v1beta1/genesis_test.go(1 hunks)warden/x/warden/module/autocli.go(0 hunks)warden/x/warden/module/genesis.go(0 hunks)warden/x/warden/module/genesis_test.go(0 hunks)warden/x/warden/module/module.go(0 hunks)warden/x/warden/module/simulation.go(1 hunks)warden/x/warden/types/v1beta3/codec.go(0 hunks)warden/x/warden/types/v1beta3/genesis.go(0 hunks)warden/x/warden/types/v1beta3/genesis_test.go(1 hunks)
💤 Files with no reviewable changes (37)
- warden/x/sched/module/autocli.go
- warden/x/act/types/v1beta1/codec.go
- precompiles/slinky/slinky.go
- warden/x/act/types/v1beta1/genesis.go
- warden/x/warden/module/genesis.go
- warden/x/sched/types/v1beta1/codec.go
- warden/x/warden/types/v1beta3/genesis.go
- warden/testutil/sample/sample.go
- proto/warden/act/v1beta1/query.proto
- warden/x/sched/types/v1beta1/genesis.go
- cmd/wardend/cmd/commands.go
- warden/app/sim_bench_test.go
- warden/x/async/types/v1beta1/genesis.go
- warden/x/warden/types/v1beta3/codec.go
- warden/x/warden/module/genesis_test.go
- warden/x/warden/module/module.go
- warden/x/act/module/genesis_test.go
- warden/x/act/module/module.go
- warden/testutil/network/network.go
- cmd/wardend/cmd/config.go
- warden/app/config.go
- warden/x/sched/module/genesis_test.go
- warden/app/legacy.go
- warden/app/app_config.go
- warden/x/async/module/genesis.go
- warden/x/sched/module/module.go
- warden/x/async/module/genesis_test.go
- warden/testutil/nullify/nullify.go
- warden/app/sim_test.go
- warden/app/ante.go
- warden/x/async/module/module.go
- warden/app/genesis.go
- warden/x/act/module/autocli.go
- warden/x/sched/module/genesis.go
- warden/x/warden/module/autocli.go
- cmd/wardend/cmd/wait.go
- warden/x/act/module/genesis.go
🚧 Files skipped from review as they are similar to previous changes (26)
- warden/x/sched/module/simulation.go
- CHANGELOG.md
- warden/app/ante/sched_handler.go
- warden/x/sched/types/v1beta1/genesis_test.go
- warden/app/upgrades.go
- warden/app/vemanager/vemanager.go
- precompiles/act/act.go
- tests/framework/exec/warden_eth.go
- cmd/wardend/config/evm_app_options.go
- warden/x/act/types/v1beta1/genesis_test.go
- tests/framework/snapshot/snapshots.go
- cmd/wardend/config/chain_id.go
- warden/x/warden/types/v1beta3/genesis_test.go
- warden/app/ante/ante.go
- warden/x/async/module/simulation.go
- cmd/wardend/config/config.go
- warden/x/act/module/simulation.go
- precompiles/async/async.go
- warden/app/ante/options.go
- precompiles/json/json.go
- warden/x/async/types/v1beta1/keys.go
- warden/app/export.go
- warden/app/ante/cosmos.go
- warden/app/ante/evm_handler.go
- warden/x/warden/module/simulation.go
- cmd/wardend/main.go
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-11-02T10:21:03.096Z
Learnt from: backsapc
PR: warden-protocol/wardenprotocol#975
File: warden/app/app.go:562-562
Timestamp: 2024-11-02T10:21:03.096Z
Learning: In `warden/app/app.go`, suggestions to handle errors from `ParamsKeeper.GetSubspace()` may be considered out of scope.
Applied to files:
cmd/wardend/cmd/testnet.go
📚 Learning: 2024-11-21T11:35:35.678Z
Learnt from: Pitasi
PR: warden-protocol/wardenprotocol#1095
File: warden/app/oracle.go:0-0
Timestamp: 2024-11-21T11:35:35.678Z
Learning: In the Go file `warden/app/oracle.go`, it is acceptable to panic inside the goroutine that starts the oracle client, because an error here indicates a misconfiguration (e.g., the address was misconfigured), and the application should exit.
Applied to files:
warden/app/oracle.go
📚 Learning: 2024-06-10T19:20:04.350Z
Learnt from: Pitasi
PR: warden-protocol/wardenprotocol#0
File: :0-0
Timestamp: 2024-06-10T19:20:04.350Z
Learning: Pitasi has implemented error handling in the `Expand` method of `warden/x/warden/keeper/shield.go`, replacing `panic` with error returns.
Applied to files:
cmd/wardend/cmd/root.go
🧬 Code graph analysis (9)
cmd/wardend/config/wardend_config.go (1)
cmd/wardend/config/config.go (1)
EVMChainID(30-30)
precompiles/sched/sched.go (5)
precompiles/act/act.go (2)
NewPrecompile(45-70)Precompile(31-37)precompiles/async/async.go (2)
NewPrecompile(45-70)Precompile(31-37)precompiles/warden/warden.go (4)
NewPrecompile(50-78)Precompile(34-42)Precompile(81-83)Precompile(200-235)warden/x/sched/keeper/keeper.go (1)
Keeper(22-34)warden/x/sched/types/v1beta1/expected_keepers.go (1)
BankKeeper(18-22)
prophet/registry.go (1)
prophet/plugins.go (1)
Plugin(12-19)
warden/app/oracle.go (1)
warden/app/app.go (1)
App(184-255)
precompiles/warden/warden.go (2)
precompiles/act/act.go (3)
Precompile(31-37)LoadABI(41-43)NewPrecompile(45-70)precompiles/async/async.go (3)
Precompile(31-37)LoadABI(41-43)NewPrecompile(45-70)
cmd/wardend/cmd/root.go (5)
warden/app/app.go (2)
NewApp(257-1025)App(184-255)cmd/wardend/config/config.go (2)
EVMChainID(30-30)BaseDenom(26-26)cmd/wardend/config/wardend_config.go (2)
MustGetDefaultNodeHome(34-41)InitAppConfig(122-186)cmd/wardend/config/evm_app_options.go (1)
EvmAppOptions(25-44)cmd/wardend/config/chain_id.go (1)
GetChainIDFromHome(12-29)
warden/app/app.go (18)
warden/x/act/keeper/keeper.go (2)
Keeper(21-38)NewKeeper(48-91)warden/x/async/keeper/keeper.go (2)
Keeper(34-61)NewKeeper(79-145)warden/x/warden/keeper/keeper.go (2)
Keeper(18-45)NewKeeper(62-122)warden/x/sched/keeper/keeper.go (2)
Keeper(22-34)NewKeeper(44-76)prophet/prophet.go (2)
P(22-31)New(34-51)prophet/plugins/pfp/pfp.go (2)
New(25-40)Plugin(20-23)prophet/plugins/veniceimg/veniceimg.go (2)
New(21-30)Plugin(17-19)prophet/plugins/quantkit/quantkit.go (2)
New(18-30)Plugin(13-15)prophet/plugins/venice/venice.go (2)
New(15-24)Plugin(11-13)warden/x/act/types/v1beta1/registry.go (2)
NewTemplatesRegistry(68-72)Register(37-44)warden/app/precompiles.go (1)
NewAvailableStaticPrecompiles(82-211)warden/app/genesis.go (3)
GenesisState(19-19)NewEVMGenesisState(22-27)NewErc20GenesisState(33-39)warden/app/ante/options.go (1)
HandlerOptions(15-23)warden/app/ante/ante.go (1)
NewAnteHandler(17-58)prophet/registry.go (1)
Register(19-28)prophet/plugins/http/http.go (2)
Plugin(25-30)NewPlugin(33-51)prophet/plugins/echo/echo.go (1)
Plugin(13-13)prophet/plugins/pricepred/pricepred.go (1)
NewPricePredictorSolidity(22-30)
precompiles/precompiles.go (6)
warden/x/async/types/v1beta1/expected_keepers.go (1)
BankKeeper(31-35)warden/x/sched/types/v1beta1/expected_keepers.go (1)
BankKeeper(18-22)precompiles/act/act.go (1)
NewPrecompile(45-70)precompiles/async/async.go (1)
NewPrecompile(45-70)precompiles/sched/sched.go (1)
NewPrecompile(46-73)precompiles/warden/warden.go (1)
NewPrecompile(50-78)
warden/app/precompiles.go (5)
precompiles/act/act.go (1)
NewPrecompile(45-70)precompiles/async/async.go (1)
NewPrecompile(45-70)precompiles/sched/sched.go (1)
NewPrecompile(46-73)precompiles/warden/warden.go (1)
NewPrecompile(50-78)precompiles/slinky/slinky.go (1)
NewPrecompile(47-70)
🪛 OSV Scanner (2.1.0)
tests/slinky/go.mod
[CRITICAL] 45-45: github.com/btcsuite/btcd 0.22.1: Denial of service in message decoding in github.com/btcsuite/btcd
(GO-2022-1098)
[CRITICAL] 45-45: github.com/btcsuite/btcd 0.22.1: Consensus failures in github.com/btcsuite/btcd
(GO-2024-2818)
[CRITICAL] 45-45: github.com/btcsuite/btcd 0.22.1: Consensus failure in github.com/btcsuite/btcd
(GO-2024-3189)
[CRITICAL] 45-45: github.com/btcsuite/btcd 0.22.1: btcd did not correctly re-implement Bitcoin Core's "FindAndDelete()" functionality
[CRITICAL] 45-45: github.com/btcsuite/btcd 0.22.1: btcd mishandles witness size checking
[CRITICAL] 45-45: github.com/btcsuite/btcd 0.22.1: btcd susceptible to consensus failures
[HIGH] 51-51: github.com/cosmos/cosmos-sdk 0.50.11: The x/crisis package does not cause chain halt in github.com/cosmos/cosmos-sdk
(GO-2023-1821)
[HIGH] 51-51: github.com/cosmos/cosmos-sdk 0.50.11: The x/crisis package does not charge ConstantFee in github.com/cosmos/cosmos-sdk
(GO-2023-1881)
[HIGH] 51-51: github.com/cosmos/cosmos-sdk 0.50.11: Cosmos SDK: Groups module can halt chain when handling a malicious proposal in github.com/cosmos/cosmos-sdk
(GO-2025-3476)
[HIGH] 51-51: github.com/cosmos/cosmos-sdk 0.50.11: Cosmos SDK can halt when erroring in EndBlocker in github.com/cosmos/cosmos-sdk/
(GO-2025-3516)
[HIGH] 51-51: github.com/cosmos/cosmos-sdk 0.50.11: Integer Overflow vulnerability in its Validator Rewards pool can cause a chain halt in github.com/cosmos/cosmos-sdk
(GO-2025-3803)
[HIGH] 51-51: github.com/cosmos/cosmos-sdk 0.50.11: Cosmos SDK: x/group can halt when erroring in EndBlocker
[HIGH] 51-51: github.com/cosmos/cosmos-sdk 0.50.11: Cosmos SDK's Integer Overflow vulnerability in its Validator Rewards pool can cause a chain halt
[HIGH] 51-51: github.com/cosmos/cosmos-sdk 0.50.11: Cosmos SDK: Groups module can halt chain when handling a malicious proposal
⏰ 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). (8)
- GitHub Check: lint
- GitHub Check: test
- GitHub Check: lint
- GitHub Check: test
- GitHub Check: lint
- GitHub Check: test
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (27)
prophet/registry.go (1)
10-11: LGTM: explicit map initPreallocating the plugins map at package init is correct and concurrency-safe with the RWMutex.
localnet.just (2)
69-69: LGTM: enable no_base_fee for local devThis aligns with frictionless local testing and your updated tests/framework.
71-76: Sleep runs only when deploying; confirm intent or move before early returnIf the wait is desired regardless of deployment, place it before the guard.
post_start () { - if [[ "{{deploy-contracts}}" == "false" ]]; then - return - fi - - sleep 5 + sleep 5 + if [[ "{{deploy-contracts}}" == "false" ]]; then + return + fitests/slinky/slinky_test.go (2)
27-28: Field rename to UIDGID looks correct.Matches upstream interchaintest/v8 change; no functional risk.
37-38: Consistent UIDGID usage for oracle image.Good alignment with new API.
warden/x/sched/keeper/abci.go (3)
21-21: New crypto import is appropriate for deriving tx.From.OK to use crypto.PubkeyToAddress for setting From.
138-141: Validate gas headroom for precompile call.200_000 should exceed intrinsic cost, but if executeCallbacks grows, this could underflow at runtime. Consider a named const with a comment on budgeting assumptions or estimating intrinsic gas during tests.
166-170: ConfirmFromEthereumTxerror semantics; don’t ignore a possible error.
Thergsearch on your module cache didn’t locateMsgEthereumTx.FromEthereumTx, so please manually verify that in your pinned EVM implementation it has the signaturefunc (m *MsgEthereumTx) FromEthereumTx(*types.Transaction) errorand handle its error return accordingly:
func buildCosmosWrappedTx(builder client.TxBuilder, signedEthTx *ethtypes.Transaction) (signing.Tx, error) { - tx.FromEthereumTx(signedEthTx) + if err := tx.FromEthereumTx(signedEthTx); err != nil { + return nil, fmt.Errorf("FromEthereumTx: %w", err) + } tx.From = crypto.PubkeyToAddress(privkey.PublicKey).Bytes()warden/app/oracle.go (1)
58-61: Verify nil mempool usage in DefaultProposalHandler
Passingnilas the mempool toNewDefaultProposalHandlermay impact proposer selection, prioritization, or recheck logic. Manually confirm that no code path in the handler assumes a non-nil mempool.justfile (1)
27-30: Ignore fee/gas mismatch warning
Localnet and e2e snapshots configureminimum-gas-prices = 0award, so--gas 10000000 --fees 1awardyields a gas price > 0 and will not be rejected.Likely an incorrect or invalid review comment.
go.mod (1)
3-3: Build images use Go 1.25
Dockerfiles (warden.Dockerfile and all cmd/*/Dockerfile) reference golang:1.25-trixie, matching thego 1.25.0directive.tests/slinky/go.mod (2)
3-3: CI uses Go 1.25, so the go 1.24 directive is supported. The GitHub workflows all invoke actions/setup-go@v5 with go-version “1.25,” so no changes are needed to the go directive.
39-39: Cosmossdk.io/depinject is only a transitive dependency – no direct imports found.warden/app/precompiles.go (4)
40-56: Options pattern for codecs: solid and backward-compatible.Good defaulting and selective overrides; keeps constructors stable.
116-134: Staking/distribution codec injection: correct usage.Passing AddressCodec into staking and distribution precompiles is consistent with new constructors.
154-171: Gov/slashing constructor updates: correct and aligned with upstream.Bank keeper and codec wiring look right; Validator/Consensus codecs injected.
188-196: Bank-aware Warden precompiles: correct threading.bankKeeper now provided; matches precompile constructors elsewhere.
warden/app/app.go (3)
1289-1291: GetAnteHandler delegates to BaseApp’s AnteHandler: OK.Accessor is correct; no action.
1302-1318: Graceful Close joins mempool and baseapp errors: good shutdown behavior.LGTM.
1482-1504: NewTxConfig custom signers and Bech32 codecs: coherent with EVM/erc20 flows.LGTM.
precompiles/sched/sched.go (1)
46-71: Add bankKeeper and wire balance handler: correct enhancement.Constructor change is minimal and matches the precompile contract pattern.
precompiles/warden/warden.go (2)
35-41: Good migration to evmcmn.Precompile and typed fields.Embedding evmcmn.Precompile and wiring the query servers look correct and consistent with other precompiles.
111-116: Gas error handling LGTM.Switch to evmcmn.HandleGasError is correct for the updated common.
cmd/wardend/config/wardend_config.go (1)
34-41: MustGetDefaultNodeHome semantics LGTM.“Must*” behavior (panic on error) is acceptable here.
precompiles/precompiles.go (1)
37-44: BankKeeper propagation and balance handler wiring LGTM.Signature changes and constructor calls are consistent across act/warden/async/sched and match the new common.
Also applies to: 48-52, 65-70, 101-105, 121-121
cmd/wardend/cmd/root.go (2)
125-147: Client context setup flow LGTM.Online textual sign mode is conditionally enabled and context is propagated correctly.
187-193: Bech32 prefixes are initialized before sealing
The setupSDKConfig in cmd/wardend/main.go calls wardendconfig.SetBech32Prefixes(config) then config.Seal(), so the prefixes are set globally prior to the Seal() in initRootCmd.
| blockedPrecompilesHex := evmtypes.AvailableStaticPrecompiles | ||
| for _, addr := range corevm.PrecompiledAddressesBerlin { | ||
| blockedPrecompilesHex = append(blockedPrecompilesHex, addr.Hex()) | ||
| } | ||
|
|
||
| for _, precompile := range blockedPrecompilesHex { | ||
| blockedAddrs[cosmosevmutils.Bech32StringFromHexAddress(precompile)] = true | ||
| } |
There was a problem hiding this comment.
Bug: possible mutation of evmtypes.AvailableStaticPrecompiles via slice aliasing.
Assigning the package slice and appending may modify the source slice if capacity permits. Copy before appending.
- blockedPrecompilesHex := evmtypes.AvailableStaticPrecompiles
- for _, addr := range corevm.PrecompiledAddressesBerlin {
- blockedPrecompilesHex = append(blockedPrecompilesHex, addr.Hex())
- }
+ blockedPrecompilesHex := make([]string, 0, len(evmtypes.AvailableStaticPrecompiles)+len(corevm.PrecompiledAddressesBerlin))
+ blockedPrecompilesHex = append(blockedPrecompilesHex, evmtypes.AvailableStaticPrecompiles...)
+ for _, addr := range corevm.PrecompiledAddressesBerlin {
+ blockedPrecompilesHex = append(blockedPrecompilesHex, addr.Hex())
+ }📝 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.
| blockedPrecompilesHex := evmtypes.AvailableStaticPrecompiles | |
| for _, addr := range corevm.PrecompiledAddressesBerlin { | |
| blockedPrecompilesHex = append(blockedPrecompilesHex, addr.Hex()) | |
| } | |
| for _, precompile := range blockedPrecompilesHex { | |
| blockedAddrs[cosmosevmutils.Bech32StringFromHexAddress(precompile)] = true | |
| } | |
| blockedPrecompilesHex := make([]string, 0, len(evmtypes.AvailableStaticPrecompiles)+len(corevm.PrecompiledAddressesBerlin)) | |
| blockedPrecompilesHex = append(blockedPrecompilesHex, evmtypes.AvailableStaticPrecompiles...) | |
| for _, addr := range corevm.PrecompiledAddressesBerlin { | |
| blockedPrecompilesHex = append(blockedPrecompilesHex, addr.Hex()) | |
| } | |
| for _, precompile := range blockedPrecompilesHex { | |
| blockedAddrs[cosmosevmutils.Bech32StringFromHexAddress(precompile)] = true | |
| } |
🤖 Prompt for AI Agents
In cmd/wardend/config/wardend_config.go around lines 85 to 92, the code assigns
evmtypes.AvailableStaticPrecompiles to blockedPrecompilesHex and then appends to
it, which can mutate the package-level slice via slice aliasing; make a copy of
the source slice before modifying it (e.g., allocate a new slice with the same
length/capacity and copy the elements, or initialize via append([]string{},
evmtypes.AvailableStaticPrecompiles...)), then append the
corevm.PrecompiledAddressesBerlin hex strings to that new slice and proceed to
populate blockedAddrs from the copied slice.
| EVM cosmosevmserverconfig.EVMConfig | ||
| JSONRPC cosmosevmserverconfig.JSONRPCConfig | ||
| TLS cosmosevmserverconfig.TLSConfig | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add mapstructure/json tags for nested EVM/JSONRPC/TLS configs.
Hyphenated keys (json-rpc) won’t map reliably without tags.
- EVM cosmosevmserverconfig.EVMConfig
- JSONRPC cosmosevmserverconfig.JSONRPCConfig
- TLS cosmosevmserverconfig.TLSConfig
+ EVM cosmosevmserverconfig.EVMConfig `mapstructure:"evm" json:"evm"`
+ JSONRPC cosmosevmserverconfig.JSONRPCConfig `mapstructure:"json-rpc" json:"json-rpc"`
+ TLS cosmosevmserverconfig.TLSConfig `mapstructure:"tls" json:"tls"`📝 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.
| EVM cosmosevmserverconfig.EVMConfig | |
| JSONRPC cosmosevmserverconfig.JSONRPCConfig | |
| TLS cosmosevmserverconfig.TLSConfig | |
| // … | |
| EVM cosmosevmserverconfig.EVMConfig `mapstructure:"evm" json:"evm"` | |
| JSONRPC cosmosevmserverconfig.JSONRPCConfig `mapstructure:"json-rpc" json:"json-rpc"` | |
| TLS cosmosevmserverconfig.TLSConfig `mapstructure:"tls" json:"tls"` | |
| // … |
🤖 Prompt for AI Agents
In cmd/wardend/config/wardend_config.go around lines 107-110, the nested fields
EVM, JSONRPC and TLS lack mapstructure/json tags so hyphenated keys like
"json-rpc" won't map; add explicit struct tags to each field specifying both
json and mapstructure names (e.g. for EVM use json:"evm" mapstructure:"evm", for
JSONRPC use json:"json-rpc" mapstructure:"json-rpc", for TLS use json:"tls"
mapstructure:"tls") so the config loader/unmarshal correctly maps nested
configs.
| customAppTemplate := serverconfig.DefaultConfigTemplate + | ||
| oracleconfig.DefaultConfigTemplate + | ||
| cosmosevmserverconfig.DefaultEVMConfigTemplate + | ||
| pricepredconfig.DefaultConfigTemplate + | ||
| httpconfig.DefaultConfigTemplate + | ||
| pfpconfig.DefaultConfigTemplate + | ||
| quantkitconfig.DefaultConfigTemplate + | ||
| veniceconfig.DefaultConfigTemplate + | ||
| veniceimgconfig.DefaultConfigTemplate | ||
|
|
There was a problem hiding this comment.
Missing JSON-RPC and TLS templates in customAppTemplate.
Without these, app.toml will omit json-rpc/tls sections despite being populated in customAppConfig.
customAppTemplate := serverconfig.DefaultConfigTemplate +
oracleconfig.DefaultConfigTemplate +
cosmosevmserverconfig.DefaultEVMConfigTemplate +
+ cosmosevmserverconfig.DefaultJSONRPCConfigTemplate +
+ cosmosevmserverconfig.DefaultTLSConfigTemplate +
pricepredconfig.DefaultConfigTemplate +
httpconfig.DefaultConfigTemplate +
pfpconfig.DefaultConfigTemplate +
quantkitconfig.DefaultConfigTemplate +
veniceconfig.DefaultConfigTemplate +
veniceimgconfig.DefaultConfigTemplate📝 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.
| customAppTemplate := serverconfig.DefaultConfigTemplate + | |
| oracleconfig.DefaultConfigTemplate + | |
| cosmosevmserverconfig.DefaultEVMConfigTemplate + | |
| pricepredconfig.DefaultConfigTemplate + | |
| httpconfig.DefaultConfigTemplate + | |
| pfpconfig.DefaultConfigTemplate + | |
| quantkitconfig.DefaultConfigTemplate + | |
| veniceconfig.DefaultConfigTemplate + | |
| veniceimgconfig.DefaultConfigTemplate | |
| customAppTemplate := serverconfig.DefaultConfigTemplate + | |
| oracleconfig.DefaultConfigTemplate + | |
| cosmosevmserverconfig.DefaultEVMConfigTemplate + | |
| cosmosevmserverconfig.DefaultJSONRPCConfigTemplate + | |
| cosmosevmserverconfig.DefaultTLSConfigTemplate + | |
| pricepredconfig.DefaultConfigTemplate + | |
| httpconfig.DefaultConfigTemplate + | |
| pfpconfig.DefaultConfigTemplate + | |
| quantkitconfig.DefaultConfigTemplate + | |
| veniceconfig.DefaultConfigTemplate + | |
| veniceimgconfig.DefaultConfigTemplate |
🤖 Prompt for AI Agents
In cmd/wardend/config/wardend_config.go around lines 175 to 184,
customAppTemplate concatenation is missing the JSON-RPC and TLS config
templates; update the concatenation to include the appropriate
DefaultConfigTemplate variables for the JSON-RPC and TLS packages (e.g.
jsonrpcconfig.DefaultConfigTemplate and tlsconfig.DefaultConfigTemplate) so the
app.toml will include those sections, and add the corresponding import entries
at the top of the file if they are not already imported.
| // Register registers a new task plugin with the given unique name. | ||
| func Register(name string, task Plugin) { | ||
| registry.rw.Lock() | ||
| defer registry.rw.Unlock() | ||
|
|
||
| if _, found := registry.tasks[name]; found { | ||
| panic("task already registered") | ||
| if _, found := registry.plugins[name]; found { | ||
| panic(fmt.Errorf("plugin %s registered twice", name)) | ||
| } | ||
|
|
||
| registry.tasks[name] = task | ||
| registry.plugins[name] = task | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden Register: naming consistency + input validation
- Rename param
task→pluginfor consistency. - Guard against empty name and nil plugin.
- Use Sprintf (or a string) instead of creating an error only to panic it.
-// Register registers a new task plugin with the given unique name.
+// Register registers a new plugin with the given unique name.
-func Register(name string, task Plugin) {
+func Register(name string, plugin Plugin) {
registry.rw.Lock()
defer registry.rw.Unlock()
- if _, found := registry.plugins[name]; found {
- panic(fmt.Errorf("plugin %s registered twice", name))
- }
-
- registry.plugins[name] = task
+ if name == "" {
+ panic("plugin name cannot be empty")
+ }
+ if _, found := registry.plugins[name]; found {
+ panic(fmt.Sprintf("plugin %q registered twice", name))
+ }
+ if plugin == nil {
+ panic("plugin " + name + " is nil")
+ }
+ registry.plugins[name] = plugin
}📝 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.
| // Register registers a new task plugin with the given unique name. | |
| func Register(name string, task Plugin) { | |
| registry.rw.Lock() | |
| defer registry.rw.Unlock() | |
| if _, found := registry.tasks[name]; found { | |
| panic("task already registered") | |
| if _, found := registry.plugins[name]; found { | |
| panic(fmt.Errorf("plugin %s registered twice", name)) | |
| } | |
| registry.tasks[name] = task | |
| registry.plugins[name] = task | |
| } | |
| // Register registers a new plugin with the given unique name. | |
| func Register(name string, plugin Plugin) { | |
| registry.rw.Lock() | |
| defer registry.rw.Unlock() | |
| if name == "" { | |
| panic("plugin name cannot be empty") | |
| } | |
| if _, found := registry.plugins[name]; found { | |
| panic(fmt.Sprintf("plugin %q registered twice", name)) | |
| } | |
| if plugin == nil { | |
| panic("plugin " + name + " is nil") | |
| } | |
| registry.plugins[name] = plugin | |
| } |
🤖 Prompt for AI Agents
In prophet/registry.go around lines 18 to 28, rename the parameter `task` to
`plugin` for naming consistency, add input validation to panic when `name` is
empty or `plugin` is nil (check before mutating registry), and replace
panic(fmt.Errorf(...)) with panic(fmt.Sprintf("plugin %s registered twice",
name)) or simply panic("...") to avoid creating an error only to panic it;
preserve the existing rw lock/unlock and registry assignment semantics.
ae80e60 to
a9875fe
Compare
Pitasi
force-pushed
the
remove-depinject
branch
from
a9875fe to
5f3e20e
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
warden/app/export.go (3)
65-71: Avoid fatal exit during genesis export; return an error.
Using log.Fatal here terminates the process abruptly. Return a wrapped error so callers can handle it.- _, err := sdk.ValAddressFromBech32(addr) - if err != nil { - log.Fatal(err) - } + if _, err := sdk.ValAddressFromBech32(addr); err != nil { + return fmt.Errorf("invalid jailed-allowlist validator address %q: %w", addr, err) + }
89-98: Replace panics and unchecked returns with proper error handling in rewards/delegation re-init.
Panics during export are undesirable; also ignore-errors can mask state issues.- for _, delegation := range dels { - valAddr, err := sdk.ValAddressFromBech32(delegation.ValidatorAddress) - if err != nil { - panic(err) - } - delAddr := sdk.MustAccAddressFromBech32(delegation.DelegatorAddress) - _, _ = app.DistrKeeper.WithdrawDelegationRewards(ctx, delAddr, valAddr) - } + for _, delegation := range dels { + valAddr, err := sdk.ValAddressFromBech32(delegation.ValidatorAddress) + if err != nil { + return fmt.Errorf("parse validator addr %q: %w", delegation.ValidatorAddress, err) + } + delAddr, err := sdk.AccAddressFromBech32(delegation.DelegatorAddress) + if err != nil { + return fmt.Errorf("parse delegator addr %q: %w", delegation.DelegatorAddress, err) + } + if _, err := app.DistrKeeper.WithdrawDelegationRewards(ctx, delAddr, valAddr); err != nil { + return fmt.Errorf("withdraw delegation rewards (%s -> %s): %w", delAddr, valAddr, err) + } + } @@ - valAddr, err := sdk.ValAddressFromBech32(del.ValidatorAddress) - if err != nil { - panic(err) - } - delAddr := sdk.MustAccAddressFromBech32(del.DelegatorAddress) + valAddr, err := sdk.ValAddressFromBech32(del.ValidatorAddress) + if err != nil { + return fmt.Errorf("parse validator addr %q: %w", del.ValidatorAddress, err) + } + delAddr, err := sdk.AccAddressFromBech32(del.DelegatorAddress) + if err != nil { + return fmt.Errorf("parse delegator addr %q: %w", del.DelegatorAddress, err) + } @@ - if err := app.DistrKeeper.Hooks().BeforeDelegationCreated(ctx, delAddr, valAddr); err != nil { - // never called as BeforeDelegationCreated always returns nil - panic(fmt.Errorf("error while incrementing period: %w", err)) - } + if err := app.DistrKeeper.Hooks().BeforeDelegationCreated(ctx, delAddr, valAddr); err != nil { + return fmt.Errorf("incrementing period: %w", err) + } @@ - if err := app.DistrKeeper.Hooks().AfterDelegationModified(ctx, delAddr, valAddr); err != nil { - // never called as AfterDelegationModified always returns nil - panic(fmt.Errorf("error while creating a new delegation period record: %w", err)) - } + if err := app.DistrKeeper.Hooks().AfterDelegationModified(ctx, delAddr, valAddr); err != nil { + return fmt.Errorf("creating delegation period record: %w", err) + }Also applies to: 100-118
230-233: Avoid log.Fatal on ApplyAndReturnValidatorSetUpdates; return error.
Terminate-without-context hurts UX and automation.- if err != nil { - log.Fatal(err) - } + if err != nil { + return fmt.Errorf("apply validator set updates: %w", err) + }
♻️ Duplicate comments (20)
warden/x/sched/keeper/abci.go (1)
36-38: Don't hard-fail on unexpected injected-tx position; warn and proceed.Strict equality to position 2 is brittle and can cause avoidable block rejections.
- // expect the tx to be at the block position [schedTxTriggerPosition] - if position != schedTxTriggerPosition { - ctx.Logger().Error("found x/sched injected transaction at the wrong position in block", "expected", schedTxTriggerPosition, "found", position) - - return &cometabci.ResponseProcessProposal{ - Status: cometabci.ResponseProcessProposal_REJECT, - }, nil - } + // prefer the tx at [schedTxTriggerPosition], but don't make it fatal + if position != schedTxTriggerPosition { + ctx.Logger().Warn("x/sched injected transaction found at unexpected position; proceeding", + "expected", schedTxTriggerPosition, "found", position) + }Also applies to: 94-101
warden/app/oracle.go (1)
109-119: Oracle client start panic is acceptable hereStarting the oracle client in a goroutine and panicking on misconfiguration aligns with prior guidance for this file.
Using retrieved learnings noted for oracle.go.
go.mod (1)
380-381: Drop non-canonical YAML moduleUse gopkg.in/yaml.v2 only; remove go.yaml.in/yaml/v2 to avoid supply-chain confusion.
- go.yaml.in/yaml/v2 v2.4.2 // indirectjustfile (1)
33-35: Update wasmvm checksums for v3.0.2 to match upstream artifactswasmvm_version := `go list -m -f '{{ .Version }}' github.com/CosmWasm/wasmvm/v3` -wasmvm_amd64_checksum := 'f2fab51ec2b393ffe6912ff31497c6f8a14c04ad2490a1a3a9fa11a37cb4ce33' -wasmvm_arm64_checksum := 'f82e608707be7b193a78f5a7f7c6163b633a94ca6a0017a7fa3501cc8e9fbff5' +wasmvm_amd64_checksum := 'b249396cf884b207f49f46bcf5b8d1fd73b8618eebbe35afb8bf60a8bb24f30a' +wasmvm_arm64_checksum := 'b9df5056ab9f61d3f9b944060b44e893d7ade7dad6ff134b36276be0f9a4185a'Retain or add a CI check to automatically verify these checksums on release.
warden/app/export.go (2)
225-228: Return iterator Close error instead of swallowing it.
Currently logs and returns nil, hiding the failure.if err := iter.Close(); err != nil { app.Logger().Error("error while closing the key-value store reverse prefix iterator: ", err) - return nil + return fmt.Errorf("closing validator reverse iterator: %w", err) }
130-156: IterateValidators: closure errors are still lost; capture and return them.
Returning true stops iteration but drops the underlying error.- err = app.StakingKeeper.IterateValidators(ctx, func(_ int64, val stakingtypes.ValidatorI) (stop bool) { + var iterErr error + err = app.StakingKeeper.IterateValidators(ctx, func(_ int64, val stakingtypes.ValidatorI) (stop bool) { @@ - scraps, err := app.DistrKeeper.GetValidatorOutstandingRewardsCoins(ctx, sdk.ValAddress(val.GetOperator())) - if err != nil { - return true - } + scraps, err := app.DistrKeeper.GetValidatorOutstandingRewardsCoins(ctx, sdk.ValAddress(val.GetOperator())) + if err != nil { iterErr = err; return true } @@ - feePool, err := app.DistrKeeper.FeePool.Get(ctx) - if err != nil { - return true - } + feePool, err := app.DistrKeeper.FeePool.Get(ctx) + if err != nil { iterErr = err; return true } @@ - err = app.DistrKeeper.FeePool.Set(ctx, feePool) - if err != nil { - return true - } + if err = app.DistrKeeper.FeePool.Set(ctx, feePool); err != nil { iterErr = err; return true } @@ - err = app.DistrKeeper.Hooks().AfterValidatorCreated(ctx, sdk.ValAddress(val.GetOperator())) - // this lets us stop in case there's an error - return err != nil + if err = app.DistrKeeper.Hooks().AfterValidatorCreated(ctx, sdk.ValAddress(val.GetOperator())); err != nil { + iterErr = err + return true + } + return false }) if err != nil { return err } +if iterErr != nil { + return iterErr +}tests/slinky/go.mod (1)
22-26: Bump cosmos-sdk to a patched release (v0.53.4).
v0.50.11 has multiple HIGH advisories; align test module with patched series.-require ( - github.com/cosmos/cosmos-sdk v0.50.11 +require ( + github.com/cosmos/cosmos-sdk v0.53.4warden/app/ante/ante.go (1)
23-46: Route based on any supported extension option, not only the first.
Some txs carry multiple options; scan all and pick the first recognized.- txWithExtensions, ok := tx.(authante.HasExtensionOptionsTx) - if ok { + if txWithExtensions, ok := tx.(authante.HasExtensionOptionsTx); ok { opts := txWithExtensions.GetExtensionOptions() if len(opts) > 0 { - switch typeURL := opts[0].GetTypeUrl(); typeURL { - case "/" + proto.MessageName(&schedv1beta1.ExtensionOptionsCallbacks{}): - // x/sched evm transaction it's a system transaction bypasses normal checks - anteHandler = newSchedAnteHandler(options) - case "/cosmos.evm.vm.v1.ExtensionOptionsEthereumTx": - // handle as *evmtypes.MsgEthereumTx - anteHandler = newMonoEVMAnteHandler(options) - case "/cosmos.evm.types.v1.ExtensionOptionDynamicFeeTx": - // cosmos-sdk tx with dynamic fee extension - anteHandler = newCosmosAnteHandler(options) - default: - return ctx, errorsmod.Wrapf( - errortypes.ErrUnknownExtensionOptions, - "rejecting tx with unsupported extension option: %s", typeURL, - ) - } - - return anteHandler(ctx, tx, sim) + var matched bool + for _, opt := range opts { + switch typeURL := opt.GetTypeUrl(); typeURL { + case "/" + proto.MessageName(&schedv1beta1.ExtensionOptionsCallbacks{}): + anteHandler = newSchedAnteHandler(options) + matched = true + case "/cosmos.evm.vm.v1.ExtensionOptionsEthereumTx": + anteHandler = newMonoEVMAnteHandler(options) + matched = true + case "/cosmos.evm.types.v1.ExtensionOptionDynamicFeeTx": + anteHandler = newCosmosAnteHandler(options) + matched = true + } + if matched { + return anteHandler(ctx, tx, sim) + } + } + return ctx, errorsmod.Wrapf( + errortypes.ErrUnknownExtensionOptions, + "rejecting tx with unsupported extension options: %v", opts, + ) } }cmd/wardend/cmd/root.go (1)
62-70: Build tempApp with real EVM chain ID to avoid EIP-712/config mismatches.
Resolve from home (fallback to default) before constructing tempApp.- noOpEvmAppOptions := func(_ uint64) error { return nil } - tempApp := app.NewApp( + noOpEvmAppOptions := func(_ uint64) error { return nil } + // try to match CLI/home chain ID for correct EIP-712 config in temp app + evmForTemp := wardendconfig.EVMChainID + if home := wardendconfig.MustGetDefaultNodeHome(); home != "" { + if cid, err := wardendconfig.GetChainIDFromHome(home); err == nil { + if parsed, err := parseEVMChainID(cid); err == nil { + evmForTemp = parsed + } + } + } + tempApp := app.NewApp( log.NewNopLogger(), dbm.NewMemDB(), nil, true, simtestutil.NewAppOptionsWithFlagHome(temp), - wardendconfig.EVMChainID, + evmForTemp, noOpEvmAppOptions, )cmd/wardend/cmd/testnet.go (8)
236-239: Validate validator count early to fail fast and avoid resource exhaustionAdd a sane bound check (e.g., 1..100) before allocating slices and creating dirs.
func initTestnetFiles( @@ ) error { - if args.chainID == "" { + if args.numValidators < 1 || args.numValidators > 100 { + return fmt.Errorf("number of validators must be between 1 and 100, got %d", args.numValidators) + } + if args.chainID == "" { args.chainID = "local-4221" }
280-299: Fix compile error: cannot range over intReplace invalid range over int with a classic for loop.
- for i := range args.numValidators { + for i := 0; i < args.numValidators; i++ {
317-319: Use loopback for single-host peers (0.0.0.0 is not dialable)Peers cannot dial 0.0.0.0; use 127.0.0.1 in single-machine mode.
- if args.singleMachine { - ip = "0.0.0.0" + if args.singleMachine { + ip = "127.0.0.1"
418-419: Handle app.toml write errorsDon’t ignore WriteConfigFile errors; surface them.
- srvconfig.WriteConfigFile(filepath.Join(nodeDir, "config", "app.toml"), evmCfg) + if err := srvconfig.WriteConfigFile(filepath.Join(nodeDir, "config", "app.toml"), evmCfg); err != nil { + return err + }
481-485: Fix compile error: cannot range over int (initGenFiles)Use an indexed loop or iterate over genFiles.
- for i := range numValidators { + for i := 0; i < numValidators; i++ { if err := genDoc.SaveAs(genFiles[i]); err != nil { return err } }
501-506: Fix compile error: cannot range over int (collectGenFiles)Same issue in collectGenFiles.
- for i := range numValidators { + for i := 0; i < numValidators; i++ { if singleMachine { portOffset := i nodeConfig.RPC.ListenAddress = fmt.Sprintf("tcp://0.0.0.0:%d", rpcPortStart+portOffset) nodeConfig.P2P.ListenAddress = fmt.Sprintf("tcp://0.0.0.0:%d", p2pPortStart+portOffset) }
534-540: Persist CometBFT config (config.toml) per nodeYou mutate nodeConfig in-memory but never write it; persist to config/config.toml.
// overwrite each validator's genesis file to have a canonical genesis time if err := genutil.ExportGenesisFileWithTime(genFile, chainID, nil, appState, genTime); err != nil { return err } + // persist cometbft config.toml + if err := cmtconfig.WriteConfigFile(filepath.Join(nodeDir, "config", "config.toml"), nodeConfig); err != nil { + return err + }
558-569: Fix compile error and handle IP overflow/underflow in calculateIPReplace invalid loop and guard against crossing subnets.
func calculateIP(ip string, i int) (string, error) { ipv4 := net.ParseIP(ip).To4() if ipv4 == nil { return "", fmt.Errorf("%v: non ipv4 address", ip) } - - for range i { - ipv4[3]++ - } - - return ipv4.String(), nil + orig := (uint32(ipv4[0]) << 24) | (uint32(ipv4[1]) << 16) | (uint32(ipv4[2]) << 8) | uint32(ipv4[3]) + newIP := uint32(int64(orig) + int64(i)) + // stay within the same /24 + if (newIP>>8) != (orig>>8) { + return "", fmt.Errorf("IP address overflows /24: %s + %d", ip, i) + } + return fmt.Sprintf("%d.%d.%d.%d", byte(newIP>>24), byte(newIP>>16), byte(newIP>>8), byte(newIP)), nil }warden/app/app.go (3)
914-929: Mempool captures nil/old AnteHandler; move initialization after SetAnteHandlerInitialize EVMMempool only after ante/post handlers are set; otherwise ante checks are bypassed.
- // mempool initialization (EVM priority nonce mempool) - if evmtypes.GetChainConfig() != nil { - // TODO: Get the actual block gas limit from consensus parameters - mempoolConfig := &evmmempool.EVMMempoolConfig{ - AnteHandler: app.GetAnteHandler(), - BlockGasLimit: 100_000_000, - } - - evmMempool := evmmempool.NewExperimentalEVMMempool(app.CreateQueryContext, logger, app.EVMKeeper, app.FeeMarketKeeper, app.txConfig, app.clientCtx, mempoolConfig) - app.EVMMempool = evmMempool - - app.SetMempool(evmMempool) - checkTxHandler := evmmempool.NewCheckTxHandler(evmMempool) - app.SetCheckTxHandler(checkTxHandler) - }Then add the block right after post handler setup (see next comment).
975-1002: Insert mempool initialization after ante/post handlersEnsure the mempool uses the final AnteHandler.
app.setPostHandler() + // mempool initialization (EVM priority nonce mempool) + if evmtypes.GetChainConfig() != nil { + // TODO: derive from consensus params + mempoolConfig := &evmmempool.EVMMempoolConfig{ + AnteHandler: app.GetAnteHandler(), // now set + BlockGasLimit: 100_000_000, + } + evmMempool := evmmempool.NewExperimentalEVMMempool( + app.CreateQueryContext, logger, app.EVMKeeper, app.FeeMarketKeeper, app.txConfig, app.clientCtx, mempoolConfig, + ) + app.EVMMempool = evmMempool + app.SetMempool(evmMempool) + app.SetCheckTxHandler(evmmempool.NewCheckTxHandler(evmMempool)) + }
1049-1060: InitChainer unmarshals app state into wrong type; breaks genesisUnmarshal into map[string]json.RawMessage and pass that to InitGenesis.
func (app *App) InitChainer(ctx sdk.Context, req *abci.RequestInitChain) (*abci.ResponseInitChain, error) { - var genesisState cosmosevmtypes.GenesisState + var genesisState map[string]json.RawMessage if err := json.Unmarshal(req.AppStateBytes, &genesisState); err != nil { panic(err) } if err := app.UpgradeKeeper.SetModuleVersionMap(ctx, app.ModuleManager.GetVersionMap()); err != nil { panic(err) } - return app.ModuleManager.InitGenesis(ctx, app.appCodec, genesisState) + return app.ModuleManager.InitGenesis(ctx, app.appCodec, genesisState) }
🧹 Nitpick comments (15)
warden/x/sched/keeper/abci.go (2)
21-21: Alias geth crypto to avoid confusion with std crypto.Avoid importing go-ethereum as plain "crypto"; use an explicit alias.
- "github.com/ethereum/go-ethereum/crypto" + gethcrypto "github.com/ethereum/go-ethereum/crypto" @@ - tx.From = crypto.PubkeyToAddress(privkey.PublicKey).Bytes() + tx.From = gethcrypto.PubkeyToAddress(privkey.PublicKey).Bytes()Also applies to: 169-169
67-73: Detect the extension option anywhere, not only at index 0.More robust to scan all options; preserves current single-tx invariant.
- if hasExtOptsTx, ok := tx.(authante.HasExtensionOptionsTx); ok { - opts := hasExtOptsTx.GetExtensionOptions() - - typeURL := "/" + proto.MessageName(&types.ExtensionOptionsCallbacks{}) - if len(opts) > 0 && opts[0].TypeUrl == typeURL { + if hasExtOptsTx, ok := tx.(authante.HasExtensionOptionsTx); ok { + opts := hasExtOptsTx.GetExtensionOptions() + typeURL := "/" + proto.MessageName(&types.ExtensionOptionsCallbacks{}) + foundHere := false + for _, o := range opts { + if o.TypeUrl == typeURL { + foundHere = true + break + } + } + if foundHere { if found { // found a second transaction with the extensions ctx.Logger().Error("invalid proposal: only one transaction per block can have the extension " + typeURL)warden/x/act/module/simulation.go (6)
52-54: Prefer nil slice for ProposalMsgs; align with ProposalContents and avoid allocReturning nil is idiomatic for “no proposals” and matches ProposalContents’ nil. Simulation code ranges over both equally.
func (am AppModule) ProposalMsgs(simState module.SimulationState) []simtypes.WeightedProposalMsg { - return []simtypes.WeightedProposalMsg{} + return nil }
44-49: Simplify WeightedOperations: return nil directlyNo operations are produced; skip the zero-length allocation.
func (am AppModule) WeightedOperations(simState module.SimulationState) []simtypes.WeightedOperation { - operations := make([]simtypes.WeightedOperation, 0) - - return operations + return nil }
25-28: Remove unused accs constructionaccs is built but never used; drop to reduce noise.
- accs := make([]string, len(simState.Accounts)) - for i, acc := range simState.Accounts { - accs[i] = acc.Address.String() - }
15-21: Delete scaffold “avoid unused import” block and corresponding unused importsThese placeholders and imports are no longer needed.
-import ( - "math/rand" - - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/types/module" - simtypes "github.com/cosmos/cosmos-sdk/types/simulation" - "github.com/cosmos/cosmos-sdk/x/simulation" - - actsimulation "github.com/warden-protocol/wardenprotocol/warden/x/act/simulation" - types "github.com/warden-protocol/wardenprotocol/warden/x/act/types/v1beta1" -) +import ( + "github.com/cosmos/cosmos-sdk/types/module" + simtypes "github.com/cosmos/cosmos-sdk/types/simulation" + + types "github.com/warden-protocol/wardenprotocol/warden/x/act/types/v1beta1" +)-var ( - _ = actsimulation.FindAccount - _ = rand.Rand{} - _ = sdk.AccAddress{} - _ = simulation.MsgEntryKind -)
44-46: Fix grammar in commentMinor doc cleanup.
-// WeightedOperations returns the all the gov module operations with their respective weights. +// WeightedOperations returns all module operations with their respective weights.
39-42: Remove deprecated ProposalContents method
The project is on Cosmos SDK v0.53.4, where AppModule no longer defines ProposalContents (it was superseded by ProposalMsgs in v0.50+). You can safely delete the no-op ProposalContents implementation in warden/x/act/module/simulation.go (and likewise in the async and sched modules) to shrink your surface.warden/app/oracle.go (1)
259-265: Improve panic diagnostics in PreBlockerLog panic details and height before re-panicking to aid debugging; avoid only printing the stack.
- defer func() { - if err := recover(); err != nil { - debug.PrintStack() - panic(err) - } - }() + defer func() { + if r := recover(); r != nil { + ctx.Logger().Error("panic in PreBlocker", "panic", r, "height", ctx.BlockHeight(), "stack", string(debug.Stack())) + panic(r) + } + }()localnet.just (1)
4-5: Remove unused variablesval2 and val2_mnemonic are defined but not used. Drop them to reduce drift.
- val2 := "warden194j3dx4t7u80765k2puu3082vc39yfqnvrxqu7" - val2_mnemonic := "door blast other element embark noodle maple agent drastic bunker swarm logic wide deputy exhibit bicycle evil exile air other mesh foot tackle spot"warden/app/export.go (1)
203-223: Remove unused counter or use it to set intra-tx counters.
counter is incremented but never used; dead code.- counter := int16(0) @@ - counter++Or, if intended, assign to the validator’s intra-tx field and persist.
warden/app/ante/ante.go (2)
17-21: Validate options when constructing the ante handler.
Catch misconfiguration early.-func NewAnteHandler(options HandlerOptions) sdk.AnteHandler { +func NewAnteHandler(options HandlerOptions) sdk.AnteHandler { + if err := options.Validate(); err != nil { + // choose panic or prebuilt error-returning handler; panic is consistent with SDK builder patterns + panic(err) + }
48-56: Simplify: the type switch is redundant.
tx is already sdk.Tx by signature; directly route to Cosmos ante.- switch tx.(type) { - case sdk.Tx: - anteHandler = newCosmosAnteHandler(options) - default: - return ctx, errorsmod.Wrapf(errortypes.ErrUnknownRequest, "invalid transaction type: %T", tx) - } + anteHandler = newCosmosAnteHandler(options)cmd/wardend/cmd/root.go (1)
153-155: Don’t ignore ReadFromClientConfig error (and avoid double-read).
You read client config in PreRunE; this second call ignores errors. Either remove it or handle the error.- initClientCtx, _ = clientcfg.ReadFromClientConfig(initClientCtx) + // already read in PersistentPreRunE; remove or handle error if needed + // initClientCtx, err = clientcfg.ReadFromClientConfig(initClientCtx) + // if err != nil { return nil }cmd/wardend/cmd/testnet.go (1)
606-610: Fix typo in user-facing error message ("testnests" → "testnets")Minor but user-visible.
- "testnests directory already exists for chain-id '%s': %s, please remove or select a new --chain-id", + "testnets directory already exists for chain-id '%s': %s, please remove or select a new --chain-id",
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (65)
go.sumis excluded by!**/*.sumtests/slinky/go.sumis excluded by!**/*.sumtests/testdata/snapshot-base/config/app.toml.tmplis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/genesis.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/gentx/gentx-38d79e1aae59e234ad6a60fd92896082bf41c64e.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/gentx/gentx-6945f3e30b02b9e34673acc22a18c3688ed57a19.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/node_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/config/priv_validator_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-base/keyring-test/80bd83f2b3f9f098fe4cd54891cdc86595a6d479.addressis excluded by!tests/testdata/**tests/testdata/snapshot-base/keyring-test/alice.infois excluded by!tests/testdata/**tests/testdata/snapshot-base/keyring-test/c8af84b4401237ec5048090f46c6327332be71cf.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/app.toml.tmplis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/genesis.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/gentx/gentx-32688592d17c81e774a966370c9366b35845614e.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/gentx/gentx-c1add3510774b379cfe92d361f1aa521dc9c9fc6.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/node_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/config/priv_validator_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/2ff01d371641069bd86371069b21a11f69383a33.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/32a224cecfe0a0660c93073fbcb6afebdbffa5ab.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/461cb598a46fcc43bef9044774afd0c4d66b54c9.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/63675504137926347258e5687f45cbf2b1f87b56.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/7daa0db7bba477374f17bf0cc2fa96d344bb8258.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/bob.infois excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/f114af4db343f425c8633979922e6f668f47dbc1.addressis excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/val.infois excluded by!tests/testdata/**tests/testdata/snapshot-keychain/keyring-test/writer.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/app.toml.tmplis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/genesis.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/gentx/gentx-2093ee4e76ca7a6460e5b92915bcbe61c8137495.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/gentx/gentx-a587c87739afa2c555eadf8b225cf7f389e31fa1.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/node_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/config/priv_validator_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/056d037ac7ae938ae179115f64a160d6976fc501.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/0fbc33e8e29268d962ebf0d9db19bf2715aee1d1.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/1856bab8f08b38ece497419342ec2ff2ba79b884.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/356e83fe984f8cff0e34868a9e239f7623d77644.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/460023f64738ff322a330aaadabe989057e72a66.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/640d841942f70598dea2ecc203be2efe0754fe37.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/6b8720d45650a5f4718bb85c0b3c713be7dabf83.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/alice.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/b730c0872fcb7106b5ab7fbaca574c4582e2a287.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/bddff3c30fabae236823a53b91505b5f445f973e.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/befc8528482269aef5c988845d0e365e1de01458.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/bf4bc4afeee16236f73202e7e93aafdab879c304.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/bob.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/charlie.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/dave.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/eb45ceb51c38527501b095031fa54c1b2911f981.addressis excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/erin.infois excluded by!tests/testdata/**tests/testdata/snapshot-many-users/keyring-test/frank.infois excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/app.toml.tmplis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/genesis.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/gentx/gentx-2601681988711cb256c1068a6ae0fe35755fdb26.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/gentx/gentx-dbafff433fe3c3711b6ade63130895b84e777eb3.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/node_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/config/priv_validator_key.jsonis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/16159a91f2b78293bf6e1d0525433a6ad9fa7807.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/3005241c8fb90192342c9d58b86b434f8ba1d807.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/4596f61417e0c4fae43c201a9a0cd06e3b403ffd.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/79fce8f9a2fd6d67aba6a83f8d2b499c6b9a4eb1.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/alice.infois excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/b26f6dc33e0a93a55c3caa3dbf6e58155d6ca55f.addressis excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/bob.infois excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/dave.infois excluded by!tests/testdata/**tests/testdata/snapshot-precompiles/keyring-test/e0ace9b0c67b860bd22cffcbb08591f3cc3b6e42.addressis excluded by!tests/testdata/**
📒 Files selected for processing (79)
CHANGELOG.md(1 hunks)cmd/wardend/cmd/commands.go(0 hunks)cmd/wardend/cmd/config.go(0 hunks)cmd/wardend/cmd/root.go(1 hunks)cmd/wardend/cmd/testnet.go(1 hunks)cmd/wardend/cmd/wait.go(0 hunks)cmd/wardend/config/chain_id.go(1 hunks)cmd/wardend/config/config.go(2 hunks)cmd/wardend/config/evm_app_options.go(1 hunks)cmd/wardend/config/wardend_config.go(1 hunks)cmd/wardend/main.go(1 hunks)go.mod(9 hunks)justfile(1 hunks)localnet.just(3 hunks)precompiles/act/act.go(2 hunks)precompiles/async/async.go(2 hunks)precompiles/json/json.go(1 hunks)precompiles/precompiles.go(6 hunks)precompiles/sched/sched.go(2 hunks)precompiles/slinky/slinky.go(0 hunks)precompiles/warden/warden.go(6 hunks)prophet/registry.go(1 hunks)proto/warden/act/v1beta1/query.proto(0 hunks)tests/framework/exec/warden_eth.go(3 hunks)tests/framework/snapshot/snapshots.go(1 hunks)tests/slinky/go.mod(10 hunks)tests/slinky/slinky_test.go(3 hunks)warden/app/ante.go(0 hunks)warden/app/ante/ante.go(1 hunks)warden/app/ante/cosmos.go(1 hunks)warden/app/ante/evm_handler.go(1 hunks)warden/app/ante/options.go(1 hunks)warden/app/ante/sched_handler.go(1 hunks)warden/app/app.go(4 hunks)warden/app/app_config.go(0 hunks)warden/app/config.go(0 hunks)warden/app/export.go(7 hunks)warden/app/genesis.go(0 hunks)warden/app/legacy.go(0 hunks)warden/app/oracle.go(4 hunks)warden/app/precompiles.go(6 hunks)warden/app/sim_bench_test.go(0 hunks)warden/app/sim_test.go(0 hunks)warden/app/upgrades.go(1 hunks)warden/app/vemanager/vemanager.go(2 hunks)warden/testutil/network/network.go(0 hunks)warden/testutil/nullify/nullify.go(0 hunks)warden/testutil/sample/sample.go(0 hunks)warden/x/act/module/autocli.go(0 hunks)warden/x/act/module/genesis.go(0 hunks)warden/x/act/module/genesis_test.go(0 hunks)warden/x/act/module/module.go(0 hunks)warden/x/act/module/simulation.go(1 hunks)warden/x/act/types/v1beta1/codec.go(0 hunks)warden/x/act/types/v1beta1/genesis.go(0 hunks)warden/x/act/types/v1beta1/genesis_test.go(1 hunks)warden/x/async/module/genesis.go(0 hunks)warden/x/async/module/genesis_test.go(0 hunks)warden/x/async/module/module.go(0 hunks)warden/x/async/module/simulation.go(1 hunks)warden/x/async/types/v1beta1/genesis.go(0 hunks)warden/x/async/types/v1beta1/keys.go(1 hunks)warden/x/sched/keeper/abci.go(3 hunks)warden/x/sched/module/autocli.go(0 hunks)warden/x/sched/module/genesis.go(0 hunks)warden/x/sched/module/genesis_test.go(0 hunks)warden/x/sched/module/module.go(0 hunks)warden/x/sched/module/simulation.go(1 hunks)warden/x/sched/types/v1beta1/codec.go(0 hunks)warden/x/sched/types/v1beta1/genesis.go(0 hunks)warden/x/sched/types/v1beta1/genesis_test.go(1 hunks)warden/x/warden/module/autocli.go(0 hunks)warden/x/warden/module/genesis.go(0 hunks)warden/x/warden/module/genesis_test.go(0 hunks)warden/x/warden/module/module.go(0 hunks)warden/x/warden/module/simulation.go(1 hunks)warden/x/warden/types/v1beta3/codec.go(0 hunks)warden/x/warden/types/v1beta3/genesis.go(0 hunks)warden/x/warden/types/v1beta3/genesis_test.go(1 hunks)
💤 Files with no reviewable changes (37)
- warden/x/async/module/genesis.go
- proto/warden/act/v1beta1/query.proto
- warden/x/warden/module/autocli.go
- warden/x/async/types/v1beta1/genesis.go
- warden/app/sim_bench_test.go
- warden/x/sched/module/autocli.go
- warden/x/sched/module/genesis.go
- warden/x/act/module/genesis.go
- warden/testutil/nullify/nullify.go
- warden/app/ante.go
- warden/x/warden/types/v1beta3/codec.go
- warden/x/sched/types/v1beta1/genesis.go
- warden/x/warden/module/genesis_test.go
- warden/x/act/module/autocli.go
- precompiles/slinky/slinky.go
- warden/app/genesis.go
- warden/x/act/types/v1beta1/genesis.go
- warden/x/sched/module/genesis_test.go
- cmd/wardend/cmd/config.go
- warden/x/warden/types/v1beta3/genesis.go
- warden/app/config.go
- warden/x/async/module/module.go
- warden/x/sched/types/v1beta1/codec.go
- warden/x/act/types/v1beta1/codec.go
- warden/app/app_config.go
- warden/x/act/module/genesis_test.go
- warden/x/async/module/genesis_test.go
- warden/x/warden/module/genesis.go
- cmd/wardend/cmd/wait.go
- warden/app/legacy.go
- warden/testutil/network/network.go
- cmd/wardend/cmd/commands.go
- warden/app/sim_test.go
- warden/testutil/sample/sample.go
- warden/x/act/module/module.go
- warden/x/warden/module/module.go
- warden/x/sched/module/module.go
🚧 Files skipped from review as they are similar to previous changes (28)
- warden/x/async/types/v1beta1/keys.go
- warden/app/ante/evm_handler.go
- warden/x/sched/module/simulation.go
- tests/framework/exec/warden_eth.go
- warden/app/ante/cosmos.go
- tests/framework/snapshot/snapshots.go
- warden/x/warden/types/v1beta3/genesis_test.go
- cmd/wardend/config/config.go
- cmd/wardend/config/chain_id.go
- precompiles/async/async.go
- cmd/wardend/config/wardend_config.go
- warden/app/upgrades.go
- warden/x/sched/types/v1beta1/genesis_test.go
- warden/x/async/module/simulation.go
- precompiles/sched/sched.go
- warden/x/warden/module/simulation.go
- warden/app/vemanager/vemanager.go
- warden/app/ante/sched_handler.go
- warden/app/precompiles.go
- prophet/registry.go
- cmd/wardend/config/evm_app_options.go
- tests/slinky/slinky_test.go
- cmd/wardend/main.go
- precompiles/act/act.go
- warden/x/act/types/v1beta1/genesis_test.go
- precompiles/json/json.go
- precompiles/warden/warden.go
- precompiles/precompiles.go
🧰 Additional context used
📓 Path-based instructions (1)
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Files:
CHANGELOG.md
🧠 Learnings (4)
📚 Learning: 2024-11-02T10:21:03.096Z
Learnt from: backsapc
PR: warden-protocol/wardenprotocol#975
File: warden/app/app.go:562-562
Timestamp: 2024-11-02T10:21:03.096Z
Learning: In `warden/app/app.go`, suggestions to handle errors from `ParamsKeeper.GetSubspace()` may be considered out of scope.
Applied to files:
cmd/wardend/cmd/testnet.go
📚 Learning: 2024-06-10T19:20:04.350Z
Learnt from: Pitasi
PR: warden-protocol/wardenprotocol#0
File: :0-0
Timestamp: 2024-06-10T19:20:04.350Z
Learning: Pitasi has implemented error handling in the `Expand` method of `warden/x/warden/keeper/shield.go`, replacing `panic` with error returns.
Applied to files:
cmd/wardend/cmd/root.go
📚 Learning: 2025-02-06T08:48:57.285Z
Learnt from: mn13
PR: warden-protocol/wardenprotocol#1216
File: warden/x/async/keeper/abci.go:212-216
Timestamp: 2025-02-06T08:48:57.285Z
Learning: In the collections package from Cosmos SDK, calling Keys() on an iterator fully consumes and closes the iterator, making an explicit Close() call unnecessary.
Applied to files:
warden/app/export.go
📚 Learning: 2024-11-21T11:35:35.678Z
Learnt from: Pitasi
PR: warden-protocol/wardenprotocol#1095
File: warden/app/oracle.go:0-0
Timestamp: 2024-11-21T11:35:35.678Z
Learning: In the Go file `warden/app/oracle.go`, it is acceptable to panic inside the goroutine that starts the oracle client, because an error here indicates a misconfiguration (e.g., the address was misconfigured), and the application should exit.
Applied to files:
warden/app/oracle.go
🧬 Code graph analysis (5)
warden/app/ante/ante.go (1)
warden/app/ante/options.go (1)
HandlerOptions(15-23)
cmd/wardend/cmd/testnet.go (2)
cmd/wardend/config/config.go (1)
EVMChainID(30-30)cmd/wardend/config/evm_app_options.go (1)
EvmAppOptions(25-44)
cmd/wardend/cmd/root.go (9)
warden/app/app.go (2)
NewApp(257-1025)App(184-255)cmd/wardend/config/config.go (2)
EVMChainID(30-30)BaseDenom(26-26)cmd/wardend/config/wardend_config.go (2)
MustGetDefaultNodeHome(34-41)InitAppConfig(122-186)cmd/wardend/config/evm_app_options.go (1)
EvmAppOptions(25-44)cmd/wardend/cmd/gen-keychains.go (1)
AddGenesisKeychainCmd(14-74)cmd/wardend/cmd/gen-plugin.go (1)
AddGenesisPluginCmd(15-61)cmd/wardend/cmd/gen-slinky-markets.go (1)
AddGenesisSlinkyMarketsCmd(15-74)cmd/wardend/cmd/gen-spaces.go (1)
AddGenesisSpaceCmd(14-60)cmd/wardend/config/chain_id.go (1)
GetChainIDFromHome(12-29)
warden/app/export.go (1)
warden/app/app.go (1)
App(184-255)
warden/app/app.go (16)
warden/x/act/keeper/keeper.go (2)
Keeper(21-38)NewKeeper(48-91)warden/x/async/keeper/keeper.go (2)
Keeper(34-61)NewKeeper(79-145)warden/x/warden/keeper/keeper.go (2)
Keeper(18-45)NewKeeper(62-122)warden/x/sched/keeper/keeper.go (2)
Keeper(22-34)NewKeeper(44-76)prophet/prophet.go (2)
P(22-31)New(34-51)prophet/plugins/pfp/pfp.go (2)
New(25-40)Plugin(20-23)prophet/plugins/veniceimg/veniceimg.go (2)
New(21-30)Plugin(17-19)prophet/plugins/quantkit/quantkit.go (2)
New(18-30)Plugin(13-15)prophet/plugins/venice/venice.go (2)
New(15-24)Plugin(11-13)warden/app/genesis.go (3)
GenesisState(19-19)NewEVMGenesisState(22-27)NewErc20GenesisState(33-39)warden/app/ante/options.go (1)
HandlerOptions(15-23)warden/app/ante/ante.go (1)
NewAnteHandler(17-58)prophet/registry.go (1)
Register(19-28)prophet/plugins/http/http.go (2)
Plugin(25-30)NewPlugin(33-51)prophet/plugins/echo/echo.go (1)
Plugin(13-13)prophet/plugins/pricepred/pricepred.go (1)
NewPricePredictorSolidity(22-30)
🪛 GitHub Check: lint
warden/app/oracle.go
[failure] 316-316:
File is not properly formatted (gci)
🪛 GitHub Actions: Release Faucet development image
warden/app/oracle.go
[error] 316-316: golangci-lint: File is not properly formatted (gci). Command: golangci-lint run --timeout=10m ./warden/... ./cmd/wardend/... ./cmd/faucet/...
🪛 GitHub Actions: Release Wardend development image
warden/app/oracle.go
[error] 316-316: golangci-lint: File is not properly formatted (gci). Step failed: golangci-lint run --timeout=10m ./warden/... ./cmd/wardend/... ./cmd/faucet/... ./prophet/... ./precompiles/...
🪛 OSV Scanner (2.1.0)
tests/slinky/go.mod
[CRITICAL] 45-45: github.com/btcsuite/btcd 0.22.1: Denial of service in message decoding in github.com/btcsuite/btcd
(GO-2022-1098)
[CRITICAL] 45-45: github.com/btcsuite/btcd 0.22.1: Consensus failures in github.com/btcsuite/btcd
(GO-2024-2818)
[CRITICAL] 45-45: github.com/btcsuite/btcd 0.22.1: Consensus failure in github.com/btcsuite/btcd
(GO-2024-3189)
[CRITICAL] 45-45: github.com/btcsuite/btcd 0.22.1: btcd did not correctly re-implement Bitcoin Core's "FindAndDelete()" functionality
[CRITICAL] 45-45: github.com/btcsuite/btcd 0.22.1: btcd mishandles witness size checking
[CRITICAL] 45-45: github.com/btcsuite/btcd 0.22.1: btcd susceptible to consensus failures
[HIGH] 51-51: github.com/cosmos/cosmos-sdk 0.50.11: The x/crisis package does not cause chain halt in github.com/cosmos/cosmos-sdk
(GO-2023-1821)
[HIGH] 51-51: github.com/cosmos/cosmos-sdk 0.50.11: The x/crisis package does not charge ConstantFee in github.com/cosmos/cosmos-sdk
(GO-2023-1881)
[HIGH] 51-51: github.com/cosmos/cosmos-sdk 0.50.11: Cosmos SDK: Groups module can halt chain when handling a malicious proposal in github.com/cosmos/cosmos-sdk
(GO-2025-3476)
[HIGH] 51-51: github.com/cosmos/cosmos-sdk 0.50.11: Cosmos SDK can halt when erroring in EndBlocker in github.com/cosmos/cosmos-sdk/
(GO-2025-3516)
[HIGH] 51-51: github.com/cosmos/cosmos-sdk 0.50.11: Integer Overflow vulnerability in its Validator Rewards pool can cause a chain halt in github.com/cosmos/cosmos-sdk
(GO-2025-3803)
[HIGH] 51-51: github.com/cosmos/cosmos-sdk 0.50.11: Cosmos SDK: x/group can halt when erroring in EndBlocker
[HIGH] 51-51: github.com/cosmos/cosmos-sdk 0.50.11: Cosmos SDK's Integer Overflow vulnerability in its Validator Rewards pool can cause a chain halt
[HIGH] 51-51: github.com/cosmos/cosmos-sdk 0.50.11: Cosmos SDK: Groups module can halt chain when handling a malicious proposal
🔇 Additional comments (10)
warden/app/oracle.go (2)
58-61: Nil mempool in DefaultProposalHandler — confirm compatibilityPassing nil to baseapp.NewDefaultProposalHandler and only setting a signer extraction adapter looks intentional. Please confirm this matches SDK v0.53.4 expectations (no hidden mempool uses).
Would you like me to scan the codebase for any implicit mempool usage invoked by the proposal handler on your setup?
6-17: Ignore import formatting suggestion: imports in warden/app/oracle.go are already correctly grouped and sorted; no action required.Likely an incorrect or invalid review comment.
localnet.just (2)
69-69: LGTM: enabling no_base_fee for localnetSetting feemarket.params.no_base_fee = true aligns with snapshot/test expectations and simplifies local testing.
76-76: Minor: extended wait is sensiblesleep 5 improves stability before post-start actions. No further action.
go.mod (2)
52-53: No change needed for evmd pin:github.com/cosmos/evm/evmdis directly imported in your code (e.g., cmd/wardend/cmd/testnet.go), so retaining the explicit pseudo-version is required.
143-144: Remove residual depinject entry from go.mod
No direct imports ofcosmossdk.io/depinject; it’s retained as an indirect requirement—likely viacosmossdk.io/schema. Upgrade to Go ≥1.25, rungo mod why -m cosmossdk.io/depinjectto confirm no import chain, then remove the line and rungo mod tidy -compat=1.25.- cosmossdk.io/depinject v1.2.1 // indirectjustfile (1)
26-30: Deterministic wasm deployment pipeline: good improvementsExplicit gas/fees and stricter shell flags harden deployments. LGTM.
warden/app/export.go (1)
19-31: Good: error now propagated from prepForZeroHeightGenesis.
The early return on error prevents silent export failures.tests/slinky/go.mod (1)
67-69: Don’t upgrade btcd to v0.24.2—breaks existing dependencies
The v0.24 release removes the old github.com/btcsuite/btcd/btcec package (tests/slinky and interchaintest → tendermint still import it), so bumping to v0.24.2 causesgo mod tidyto fail. Hold at v0.22.1 until dependent modules update or you refactor imports to btcec/v2.Likely an incorrect or invalid review comment.
cmd/wardend/cmd/root.go (1)
447-461: ChainID parsing: clear and defensive.
The format validation prevents bad slices; good.
Pitasi
force-pushed
the
remove-depinject
branch
from
5f3e20e to
7f8de2d
Compare
Changes:
This PR is blocked by: cosmos/evm#416 (one integration test currently fails)
Closes ENG-448