chore: bump cosmos v0.50.14 and prepare upgrade handler #268
Conversation
mbreithecker
requested review from
christopherbrumm and
troykessler
as code owners
WalkthroughReleases metadata bumped to v2.2.0, app initialization wired to a new v2_2 upgrade package, v2_2 defines UpgradeName and a modified store loader config, and module dependencies (including Cosmos SDK) are upgraded; CHANGELOG replaced an unreleased state-machine note with a deps entry. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Node as Node/Daemon
participant App as App (Init)
participant Upgrade as x/upgrade Module
participant V22 as upgrades/v2_2
participant Store as Store Loader
App->>Upgrade: Register UpgradeHandler(name=V22.UpgradeName, handler=V22.CreateUpgradeHandler)
Upgrade-->>App: Handler registered
Note over Node,Upgrade: On startup, read pending upgrade plan
Node->>Upgrade: Query pending plan
Upgrade-->>Node: Plan(name, height)
alt name == V22.UpgradeName
App->>Store: loader = V22.CreateStoreLoader(upgradeHeight)
App->>Upgrade: SetStoreLoader(loader)
Note right of Store: Apply store upgrades at height
else name != V22.UpgradeName
Note over App,Upgrade: No v2_2 actions
end
Note over Node,Upgrade: At upgrade height
Upgrade->>App: Invoke v2_2 handler
App->>V22: Execute migration
V22-->>App: Migration complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (1)
✨ 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
go.mod (1)
380-380: Replace to KYVE fork rc1 aligns with the bump; plan to move off rc when stable is cutThe replace to v0.50.14-kyve-rc1 is consistent. Track follow-up to drop “-rc” when the final KYVE-flavored release is available.
app/upgrades/v2_2/upgrade.go (2)
18-18: Avoid global logger; keep it function-localThe file-level
var logger log.Loggerisn’t needed and can cause confusion in concurrent contexts. Prefer a local variable inside the handler function.Apply this diff:
-var logger log.Logger +// no package-level logger; use a local logger per execution contextAnd inside the closure:
- logger = sdkCtx.Logger().With("upgrade", UpgradeName) + logger := sdkCtx.Logger().With("upgrade", UpgradeName)
25-36: Improve logging semantics and error propagationCurrently “finished upgrade” is logged even if
RunMigrationsreturns an error. Also, string formatting with fmt isn’t needed for structured logs.Apply this diff:
- logger = sdkCtx.Logger().With("upgrade", UpgradeName) - logger.Info(fmt.Sprintf("performing upgrade %v", UpgradeName)) + logger := sdkCtx.Logger().With("upgrade", UpgradeName) + logger.Info("performing upgrade") // Run cosmos migrations migratedVersionMap, err := mm.RunMigrations(ctx, configurator, fromVM) // Run KYVE migrations - logger.Info(fmt.Sprintf("finished upgrade %v", UpgradeName)) - - return migratedVersionMap, err + if err != nil { + logger.Error("upgrade failed", "error", err) + return migratedVersionMap, err + } + logger.Info("finished upgrade") + return migratedVersionMap, nilMakefile (1)
7-7: Consider deriving BUILD_TIME to avoid manual editsNot blocking, but you can make BUILD_TIME default to “now” when not set, improving DX for ad-hoc builds.
Example:
-BUILD_TIME := 202507310800.00 # format [[CC]YY]MMDDhhmm[.ss] +BUILD_TIME ?= $(shell date -u +%Y%m%d%H%M.%S) # format [[CC]YY]MMDDhhmm[.ss]If you need reproducible artifacts for releases, keep pinning BUILD_TIME in release pipelines.
📜 Review details
Configuration used: CodeRabbit UI
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 (2)
go.sumis excluded by!**/*.sumgo.work.sumis excluded by!**/*.sum
📒 Files selected for processing (6)
CHANGELOG.md(1 hunks)Makefile(1 hunks)app/app.go(3 hunks)app/upgrades/v2_2/store.go(2 hunks)app/upgrades/v2_2/upgrade.go(2 hunks)go.mod(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/app.go (3)
app/upgrades/v2_2/upgrade.go (2)
UpgradeName(15-15)CreateUpgradeHandler(20-38)util/expected_keepers.go (1)
UpgradeKeeper(57-59)app/upgrades/v2_2/store.go (1)
CreateStoreLoader(9-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test / test
- GitHub Check: lint / golangci
🔇 Additional comments (10)
go.mod (3)
149-149: Indirect chi bump is fineBump to github.com/go-chi/chi/v5 v5.2.2 (indirect) is low-risk. No action needed.
354-354: oauth2 indirect bump acknowledgedgolang.org/x/oauth2 -> v0.27.0 is OK. Watch for transitive auth behavior changes in any cloud integrations during interchaintests.
25-25: Cosmos SDK v0.50.14 bump verified
- No occurrences of
v2_1remain in the module graphgo mod tidycompleted without errorsgo mod verifyconfirmed all modulesapp/upgrades/v2_2/upgrade.go (2)
1-1: Package rename to v2_2: LGTMThe rename is consistent with the app wiring and the new upgrade.
15-16: UpgradeName updated to v2.2.0: LGTMMatches Makefile VERSION and intended release.
app/upgrades/v2_2/store.go (1)
1-1: Package v2_2 rename: LGTMConsistent with the new upgrade path.
Makefile (1)
5-5: VERSION bump to v2.2.0: LGTMMatches upgrade name and module updates.
app/app.go (3)
8-8: No lingering v2_1 references detected — upgrade to v2_2 confirmedThe ripgrep scan returned “OK: no v2_1 references found,” so the import update is complete and there are no stale v2_1 usages left. Ready to merge.
415-420: Confirm v2.2.0 upgrade plan name usage– The upgrade handler is correctly wired to
v2_2.UpgradeNameandCreateUpgradeHandler.
– The constant in app/upgrades/v2_2/upgrade.go is indeed set to"v2.2.0".
– No in-repo scheduling or governance proposal code references"v2.2.0"directly.Please ensure your on‐chain upgrade proposal (or ops tooling) uses the exact plan name
"v2.2.0", otherwise the handler won’t be invoked.
427-430: Critical: Prevent mounting an empty store key in the v2_2 upgradeThe current
app/upgrades/v2_2/store.godefines a single empty string in itsAddedlist, which will cause the upgrade loader to attempt mounting a non-existent key and panic at upgrade height.• In app/app.go (around lines 427–430), wrap the loader call so it only runs when there are real store changes:
- if upgradeInfo.Name == v2_2.UpgradeName && !app.UpgradeKeeper.IsSkipHeight(upgradeInfo.Height) { - // configure store loader that checks if version == upgradeHeight and applies store upgrades - app.SetStoreLoader(v2_2.CreateStoreLoader(upgradeInfo.Height)) - } + if upgradeInfo.Name == v2_2.UpgradeName && !app.UpgradeKeeper.IsSkipHeight(upgradeInfo.Height) { + // only configure a store loader when there are actual KV-store changes + if v2_2.HasStoreUpgrades() { + app.SetStoreLoader(v2_2.CreateStoreLoader(upgradeInfo.Height)) + } + }• In app/upgrades/v2_2/store.go, replace the placeholder empty key with truly empty slices (or populate with real keys if any stores were added/removed/renamed):
var storeUpgrades = storetypes.StoreUpgrades{ Added: []string{}, // no new stores Deleted: []string{}, // no deletions Renamed: []storetypes.StoreRename{}, // no renames }• Verify intended store changes: run the following in your repo root to list all defined KV-store keys and confirm whether v2.2.0 actually adds/removes/renames any:
#!/bin/bash set -euo pipefail # find all StoreKey constants rg -nP --type=go '\bStoreKey\s*=\s*"[^"]+"' . || true # find all sdk.NewKVStoreKey usages rg -nP --type=go 'sdk\.NewKVStoreKey\("([^"]+)"\)' . || truePopulate
storeUpgradeswith any keys you discover, or leave all lists empty if there truly are no KV-store changes.
# Conflicts: # CHANGELOG.md
mbreithecker
changed the title
Summary by CodeRabbit