feat: upgrade to cosmos-sdk v0.54.3 + ibc-go v11 + cometbft v0.39#2080
Open
JayT106 wants to merge 11 commits into
Open
feat: upgrade to cosmos-sdk v0.54.3 + ibc-go v11 + cometbft v0.39#2080JayT106 wants to merge 11 commits into
JayT106 wants to merge 11 commits into
Conversation
JayT106
requested review from
calvinaco and
randy-cro
and removed request for
a team
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCoordinated upgrade: Go toolchain to 1.25.9, Cosmos SDK imports migrated to v0.54/v2 packages, IBC-Go v10→v11, App.New/keeper refactors, ICA/ICS4 middleware wiring changes, proposal fast-path and governance updates, upgrade handler/store-loader adjustments, and dependency pin refreshes. ChangesCosmos SDK v0.54 and IBC-Go v11 migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
This comment has been minimized.
This comment has been minimized.
![semgrep-code-crypto-org-chain[bot]](https://avatars.githubusercontent.com/u/78834054?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
docs/architecture/cosmos-sdk-v0.54-upgrade-plan.md (1)
22-45: ⚡ Quick winAvoid forcing persistent global git rewrites in the default setup path.
Using
git config --global ... insteadOfcan leak into unrelated repositories and surprise developers. Please either (a) document a repo-scoped config alternative first, or (b) include explicit cleanup commands in this section.Suggested doc tweak
-## Pre-merge dev setup (insteadOf redirect) +## Pre-merge dev setup (scoped redirect preferred) -Configure git to redirect: +Prefer repository-scoped redirect (run from this repo): + +```bash +git config --local url."https://github.com/JayT106/ethermint".insteadOf \ + "https://github.com/crypto-org-chain/ethermint" +git config --local url."https://github.com/JayT106/cronos-store".insteadOf \ + "https://github.com/crypto-org-chain/cronos-store" +``` + +If you use `--global`, add cleanup after PRs merge: + +```bash +git config --global --unset-all url."https://github.com/JayT106/ethermint".insteadOf +git config --global --unset-all url."https://github.com/JayT106/ethermint.git".insteadOf +git config --global --unset-all url."https://github.com/JayT106/cronos-store".insteadOf +git config --global --unset-all url."https://github.com/JayT106/cronos-store.git".insteadOf +```🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/architecture/cosmos-sdk-v0.54-upgrade-plan.md` around lines 22 - 45, The docs currently recommend using git config --global url."...".insteadOf which can leak into other repos; change the instructions to prefer a repo-scoped config using git config --local url."...".insteadOf for the two redirect entries (the existing git config --global url."https://github.com/JayT106/ethermint".insteadOf / git config --global url."https://github.com/JayT106/ethermint.git".insteadOf and the cronos-store equivalents) and keep the go env -w GOPRIVATE guidance, and if you still document the --global option include explicit cleanup commands using git config --global --unset-all for each url."...".insteadOf entry so developers can revert the global rewrites after the upstream PRs merge.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/architecture/cosmos-sdk-v0.54-upgrade-plan.md`:
- Line 202: The link at line 202 uses the floating `/sdk/next/upgrade/upgrade`
URL which can drift; update the markdown reference text "[Cosmos SDK v0.54
upgrade docs](https://cosmos-docs.mintlify.app/sdk/next/upgrade/upgrade)" to a
version-pinned v0.54 docs URL (replace `/sdk/next/` with the v0.54 path) so the
runbook always points to the stable v0.54 upgrade documentation.
- Line 11: Replace the vague version token "v11.x" for the ibc-go dependency
with the exact release "v11.0.0" in the upgrade plan; locate the table row
containing the "ibc-go" entry and update its version cell from "v11.x" to
"v11.0.0" so the document pins the baseline release for Cosmos SDK v0.54
rehearsals.
In `@go.mod`:
- Line 3: The module's Go version directive currently reads "go 1.26.3" which
conflicts with CI's pinned setup-go version 1.25.9; update the go directive in
go.mod from "go 1.26.3" to "go 1.25.9" so the module and CI toolchain match (or
alternatively update the CI setup-go pins to 1.26.3), and ensure any references
to the Go version in CI workflows (.github/workflows/*) are consistent with the
updated go.mod.
---
Nitpick comments:
In `@docs/architecture/cosmos-sdk-v0.54-upgrade-plan.md`:
- Around line 22-45: The docs currently recommend using git config --global
url."...".insteadOf which can leak into other repos; change the instructions to
prefer a repo-scoped config using git config --local url."...".insteadOf for the
two redirect entries (the existing git config --global
url."https://github.com/JayT106/ethermint".insteadOf / git config --global
url."https://github.com/JayT106/ethermint.git".insteadOf and the cronos-store
equivalents) and keep the go env -w GOPRIVATE guidance, and if you still
document the --global option include explicit cleanup commands using git config
--global --unset-all for each url."...".insteadOf entry so developers can revert
the global rewrites after the upstream PRs merge.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 436950d0-eee8-4267-8674-49af553a9d90
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (48)
.github/workflows/codeql-analysis.yml.github/workflows/lint.yml.github/workflows/sims.ymlapp/app.goapp/bench_test.goapp/export.goapp/proposal.goapp/proposal_test.goapp/sim_test.goapp/storeloader.goapp/test_helpers.goapp/upgrades.goapp/versiondb.goapp/versiondb_placeholder.gocmd/cronosd/cmd/root.gocmd/cronosd/dbmigrate/migrate.gocmd/cronosd/dbmigrate/migrate_basic_test.gocmd/cronosd/dbmigrate/migrate_dbname_test.gocmd/cronosd/dbmigrate/migrate_enhanced_test.gocmd/cronosd/dbmigrate/migrate_rocksdb_test.gocmd/cronosd/dbmigrate/migrate_test.gocmd/cronosd/dbmigrate/patch.gocmd/cronosd/dbmigrate/patch_advanced_test.gocmd/cronosd/dbmigrate/patch_integration_test.godocs/architecture/cosmos-sdk-v0.54-upgrade-plan.mdgo.modgomod2nix.tomltestutil/simapp/simapp.gox/cronos/client/cli/tx.gox/cronos/events/decoders.gox/cronos/events/events.gox/cronos/keeper/ibc.gox/cronos/keeper/keeper.gox/cronos/keeper/keeper_test.gox/cronos/keeper/mock/ibckeeper_mock.gox/cronos/keeper/params.gox/cronos/keeper/precompiles/bank.gox/cronos/keeper/precompiles/ica.gox/cronos/keeper/precompiles/relayer.gox/cronos/middleware/conversion_middleware.gox/cronos/middleware/conversion_middleware_test.gox/cronos/migrations/v2/migrate.gox/cronos/migrations/v2/migrate_test.gox/cronos/rpc/api.gox/cronos/types/interfaces.gox/cronos/types/proposal.gox/e2ee/keeper/keeper.gox/e2ee/keyring/keyring.go
💤 Files with no reviewable changes (1)
- app/proposal_test.go
JayT106
force-pushed
the
feat/cosmos-sdk-v0.54-upgrade-wip
branch
2 times, most recently
from
efa739a to
9374171
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/upgrades.go`:
- Around line 35-38: The current code calls
UpgradeKeeper.ReadUpgradeInfoFromDisk() and returns false on error, silently
skipping critical upgrade/store-loader setup; change the implementation so the
error is not swallowed: propagate or return the error (e.g., change the caller
signature to return (bool, error) or return the error up the stack) instead of
returning false, and ensure any callers of this function handle the error
appropriately; locate the call to UpgradeKeeper.ReadUpgradeInfoFromDisk and the
surrounding logic that uses upgradeInfo to wire the error propagation through or
fail fast (log/return) so disk read failures cannot be ignored.
In `@gomod2nix.toml`:
- Around line 296-306: The three cronos-store module entries in gomod2nix.toml
([mod."github.com/crypto-org-chain/cronos-store/memiavl"],
[mod."github.com/crypto-org-chain/cronos-store/store"],
[mod."github.com/crypto-org-chain/cronos-store/versiondb"]) are pinned to commit
704a4bc6440f while go.mod references commit 596a1419d7a1, causing inconsistent
dependency graphs; fix by reconciling versions so both files match—either update
the three version fields in gomod2nix.toml to the same pseudo-version that
includes 596a1419d7a1 (and update the corresponding hash values to the correct
sha256 entries from go.sum or from a regenerated gomod2nix output) or regenerate
gomod2nix.toml from the current go.mod so the module entries and hashes for the
cronos-store modules align with the commit referenced in go.mod.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1783d3a3-a5be-4c22-9a47-b29f0e352209
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (48)
.github/workflows/codeql-analysis.yml.github/workflows/lint.yml.github/workflows/sims.ymlapp/app.goapp/bench_test.goapp/export.goapp/proposal.goapp/proposal_test.goapp/sim_test.goapp/storeloader.goapp/test_helpers.goapp/upgrades.goapp/versiondb.goapp/versiondb_placeholder.gocmd/cronosd/cmd/root.gocmd/cronosd/dbmigrate/migrate.gocmd/cronosd/dbmigrate/migrate_basic_test.gocmd/cronosd/dbmigrate/migrate_dbname_test.gocmd/cronosd/dbmigrate/migrate_enhanced_test.gocmd/cronosd/dbmigrate/migrate_rocksdb_test.gocmd/cronosd/dbmigrate/migrate_test.gocmd/cronosd/dbmigrate/patch.gocmd/cronosd/dbmigrate/patch_advanced_test.gocmd/cronosd/dbmigrate/patch_integration_test.godocs/architecture/cosmos-sdk-v0.54-upgrade-plan.mdgo.modgomod2nix.tomltestutil/simapp/simapp.gox/cronos/client/cli/tx.gox/cronos/events/decoders.gox/cronos/events/events.gox/cronos/keeper/ibc.gox/cronos/keeper/keeper.gox/cronos/keeper/keeper_test.gox/cronos/keeper/mock/ibckeeper_mock.gox/cronos/keeper/params.gox/cronos/keeper/precompiles/bank.gox/cronos/keeper/precompiles/ica.gox/cronos/keeper/precompiles/relayer.gox/cronos/middleware/conversion_middleware.gox/cronos/middleware/conversion_middleware_test.gox/cronos/migrations/v2/migrate.gox/cronos/migrations/v2/migrate_test.gox/cronos/rpc/api.gox/cronos/types/interfaces.gox/cronos/types/proposal.gox/e2ee/keeper/keeper.gox/e2ee/keyring/keyring.go
💤 Files with no reviewable changes (1)
- app/proposal_test.go
✅ Files skipped from review due to trivial changes (10)
- x/cronos/migrations/v2/migrate_test.go
- cmd/cronosd/dbmigrate/migrate_basic_test.go
- app/storeloader.go
- x/cronos/keeper/mock/ibckeeper_mock.go
- x/cronos/types/proposal.go
- x/cronos/events/decoders.go
- cmd/cronosd/dbmigrate/migrate.go
- .github/workflows/sims.yml
- .github/workflows/codeql-analysis.yml
- cmd/cronosd/dbmigrate/patch_advanced_test.go
JayT106
force-pushed
the
feat/cosmos-sdk-v0.54-upgrade-wip
branch
from
d7524ed to
79983fa
Compare
JayT106
force-pushed
the
feat/cosmos-sdk-v0.54-upgrade-wip
branch
3 times, most recently
from
6a2b173 to
e8437d0
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2080 +/- ##
==========================================
- Coverage 16.87% 13.68% -3.20%
==========================================
Files 72 74 +2
Lines 6163 5102 -1061
==========================================
- Hits 1040 698 -342
+ Misses 5000 4329 -671
+ Partials 123 75 -48
🚀 New features to boost your workflow:
|
JayT106
force-pushed
the
feat/cosmos-sdk-v0.54-upgrade-wip
branch
5 times, most recently
from
0063035 to
adf8c57
Compare
Contributor
Author
|
@claude review PR |
JayT106
force-pushed
the
feat/cosmos-sdk-v0.54-upgrade-wip
branch
from
3adca1e to
afef52c
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
JayT106
force-pushed
the
feat/cosmos-sdk-v0.54-upgrade-wip
branch
from
69271ac to
cca1fe6
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/upgrades.go`:
- Line 35: The upgrade currently calls
app.SetStoreLoader(upgradetypes.UpgradeStoreLoader(upgradeInfo.Height,
&storetypes.StoreUpgrades{})) with an empty StoreUpgrades; verify that is
correct for the v0.54.3 + IBC v11 upgrade by comparing this app's registered
store keys and ModuleManager list (inspect functions/vars that call
storetypes.NewKVStoreKey/NewMemoryStoreKey/NewTransientStoreKey and the
ModuleManager instantiation in app.go) against the SDK v0.54 module store
changes; if no stores were added/removed/renamed, remove the no-op
SetStoreLoader call or leave as-is, otherwise populate the
&storetypes.StoreUpgrades{Added:[], Renamed:map[string]string{}, Deleted:[]}
with the exact module store key names that changed at the upgrade height so
UpgradeStoreLoader in upgradetypes applies the correct multistore modifications.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2657b094-f809-4c68-b57d-77bdb809f313
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (52)
.github/workflows/codeql-analysis.yml.github/workflows/lint.yml.github/workflows/sims.yml.github/workflows/test.ymlCHANGELOG.mdapp/app.goapp/bench_test.goapp/export.goapp/proposal.goapp/proposal_test.goapp/sim_test.goapp/storeloader.goapp/test_helpers.goapp/upgrades.goapp/versiondb.goapp/versiondb_placeholder.gocmd/cronosd/cmd/root.gocmd/cronosd/dbmigrate/migrate.gocmd/cronosd/dbmigrate/migrate_basic_test.gocmd/cronosd/dbmigrate/migrate_dbname_test.gocmd/cronosd/dbmigrate/migrate_enhanced_test.gocmd/cronosd/dbmigrate/migrate_rocksdb_test.gocmd/cronosd/dbmigrate/migrate_test.gocmd/cronosd/dbmigrate/patch.gocmd/cronosd/dbmigrate/patch_advanced_test.gocmd/cronosd/dbmigrate/patch_integration_test.gogo.modgomod2nix.tomlintegration_tests/configs/ibc.jsonnetnix/build_overlay.nixnix/sources.jsontestutil/simapp/simapp.gox/cronos/client/cli/tx.gox/cronos/events/decoders.gox/cronos/events/events.gox/cronos/keeper/ibc.gox/cronos/keeper/keeper.gox/cronos/keeper/keeper_test.gox/cronos/keeper/mock/ibckeeper_mock.gox/cronos/keeper/params.gox/cronos/keeper/precompiles/bank.gox/cronos/keeper/precompiles/ica.gox/cronos/keeper/precompiles/relayer.gox/cronos/middleware/conversion_middleware.gox/cronos/middleware/conversion_middleware_test.gox/cronos/migrations/v2/migrate.gox/cronos/migrations/v2/migrate_test.gox/cronos/rpc/api.gox/cronos/types/interfaces.gox/cronos/types/proposal.gox/e2ee/keeper/keeper.gox/e2ee/keyring/keyring.go
💤 Files with no reviewable changes (1)
- app/proposal_test.go
✅ Files skipped from review due to trivial changes (8)
- x/cronos/keeper/mock/ibckeeper_mock.go
- cmd/cronosd/dbmigrate/migrate_basic_test.go
- x/cronos/keeper/params.go
- CHANGELOG.md
- app/storeloader.go
- x/cronos/keeper/precompiles/ica.go
- .github/workflows/sims.yml
- x/cronos/types/interfaces.go
JayT106
force-pushed
the
feat/cosmos-sdk-v0.54-upgrade-wip
branch
2 times, most recently
from
cf46a65 to
d08ef30
Compare
- bump cosmos-sdk v0.53.4→v0.54.3, ibc-go v10→v11, cometbft v0.38→v0.39.3,
Go 1.25.8→1.25.9
- migrate cosmossdk.io/{store,log,x/evidence,x/feegrant,x/upgrade,x/tx}
standalone modules to in-tree github.com/cosmos/cosmos-sdk paths
(store/v2, log/v2, contrib/x/crisis)
- pin replaces: cosmos-sdk fork (PR crypto-org-chain#1812 staking queue optimization),
ethermint fork (PR crypto-org-chain#894), cronos-store fork (PR crypto-org-chain#57)
- app.go: drop traceStore from New(); SetCommitMultiStoreTracer removed;
ICAControllerKeeper/ICAHostKeeper/TransferKeeper as pointer; drop
subspaces from IBC/ICA/Transfer NewKeeper; gov adds
CalculateVoteResultsAndVotingPower; RegisterNodeService takes
EarliestVersion func; remove ParamKeyTable for ibcclient/transfer/ica
- ExtTxSelector: drop gasWanted arg; remove SelectTxForProposalFast (no
Noopmempool path)
- IBCConversionModule: forward SetICS4Wrapper to underlying app
- ibccallbacks v11: NewIBCMiddleware(ck, max) +
SetUnderlyingApplication + SetICS4Wrapper composition pattern
- blockstm.NewSTMRunner → txnrunner.NewSTMRunner
- AppCreator/AppExporter: drop io.Writer
- AccountKeeper.NextAccountNumber takes (ctx, AccountI)
- e2ee/keyring: ErrUnknownBacked → ErrUnknownBackend
- add v6.0.0-sdk54 upgrade handler with staking v6 migration + StoreUpgrades
- bump CI Go to 1.25.9
Plan: docs/architecture/cosmos-sdk-v0.54-upgrade-plan.md
IBC v2 routes deferred to follow-up PR.
JayT106
force-pushed
the
feat/cosmos-sdk-v0.54-upgrade-wip
branch
from
d08ef30 to
426a87a
Compare
cosmos-sdk v0.54 removed NewDefaultProposalHandlerFast, regressing per-block proposer cost for nodes running NoOp mempool (archives / non-priority validators) by re-running TxDecode + AnteHandler per tx. Reintroduce the fast path locally: when the mempool is nil or NoOp, filter req.Txs at byte-level via the cronos block-list ValidateTx and honor req.MaxTxBytes; otherwise delegate to the upstream default handler so priority mempool ordering semantics are preserved. Adds unit tests covering: non-NoOp delegation, NoOp filtering with order preservation, MaxTxBytes boundary, MaxTxBytes <= 0, nil mempool fast path, and empty req.Txs.
Five subtests duplicated the same `t.Fatal` default-handler stub and three duplicated the `acceptAll` validator. Hoist `mustNotInvoke` and `acceptAll` to the parent test scope so each subtest only states what is unique about its case. No behavior change; same six subtests still pass.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
x/cronos/keeper/ibc.go (1)
69-73: 💤 Low valueNote: Staticcheck suppressions added for telemetry calls.
The
//nolint:staticcheckdirectives have been added totelemetry.SetGaugeWithLabelscalls. This is acceptable for this migration PR, as it allows the upgrade to proceed while preserving existing telemetry behavior. However, these suppressions suggest the API may be deprecated.Consider tracking these for future cleanup in a follow-up PR once the SDK upgrade is stable.
Also applies to: 150-154
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@x/cronos/keeper/ibc.go` around lines 69 - 73, Staticcheck suppressions were added to telemetry.SetGaugeWithLabels calls (e.g., in ConvertVouchersToEvmCoins using telemetry.NewLabel) to allow the migration to proceed; keep the suppressions for now but add a short TODO comment above each suppressed call referencing a follow-up task/issue (e.g., "TODO: remove staticcheck suppression and migrate telemetry API — issue #<id>") so these instances (including the other occurrences around the 150-154 block) are tracked for cleanup in a subsequent PR.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@integration_tests/configs/ibc.jsonnet`:
- Around line 153-155: Update the Hermes compatibility setting by changing the
compat_mode value from '0.38' to '0.37' where the compat_mode key is set (ensure
the compat_mode assignment is updated to '0.37' to force wire-compat for
CometBFT v0.38); also update any nearby comment to reflect that '0.37' is used
for v0.37/v0.38 wire-compat if present.
---
Nitpick comments:
In `@x/cronos/keeper/ibc.go`:
- Around line 69-73: Staticcheck suppressions were added to
telemetry.SetGaugeWithLabels calls (e.g., in ConvertVouchersToEvmCoins using
telemetry.NewLabel) to allow the migration to proceed; keep the suppressions for
now but add a short TODO comment above each suppressed call referencing a
follow-up task/issue (e.g., "TODO: remove staticcheck suppression and migrate
telemetry API — issue #<id>") so these instances (including the other
occurrences around the 150-154 block) are tracked for cleanup in a subsequent
PR.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8a90424a-1cad-4a74-a346-976a6ca2d4fc
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (51)
.github/workflows/codeql-analysis.yml.github/workflows/lint.yml.github/workflows/sims.ymlCHANGELOG.mdapp/app.goapp/bench_test.goapp/export.goapp/proposal.goapp/proposal_test.goapp/sim_test.goapp/storeloader.goapp/test_helpers.goapp/upgrades.goapp/versiondb.goapp/versiondb_placeholder.gocmd/cronosd/cmd/root.gocmd/cronosd/dbmigrate/migrate.gocmd/cronosd/dbmigrate/migrate_basic_test.gocmd/cronosd/dbmigrate/migrate_dbname_test.gocmd/cronosd/dbmigrate/migrate_enhanced_test.gocmd/cronosd/dbmigrate/migrate_rocksdb_test.gocmd/cronosd/dbmigrate/migrate_test.gocmd/cronosd/dbmigrate/patch.gocmd/cronosd/dbmigrate/patch_advanced_test.gocmd/cronosd/dbmigrate/patch_integration_test.gogo.modgomod2nix.tomlintegration_tests/configs/ibc.jsonnetnix/build_overlay.nixnix/sources.jsontestutil/simapp/simapp.gox/cronos/client/cli/tx.gox/cronos/events/decoders.gox/cronos/events/events.gox/cronos/keeper/ibc.gox/cronos/keeper/keeper.gox/cronos/keeper/keeper_test.gox/cronos/keeper/mock/ibckeeper_mock.gox/cronos/keeper/params.gox/cronos/keeper/precompiles/bank.gox/cronos/keeper/precompiles/ica.gox/cronos/keeper/precompiles/relayer.gox/cronos/middleware/conversion_middleware.gox/cronos/middleware/conversion_middleware_test.gox/cronos/migrations/v2/migrate.gox/cronos/migrations/v2/migrate_test.gox/cronos/rpc/api.gox/cronos/types/interfaces.gox/cronos/types/proposal.gox/e2ee/keeper/keeper.gox/e2ee/keyring/keyring.go
✅ Files skipped from review due to trivial changes (11)
- x/cronos/keeper/mock/ibckeeper_mock.go
- cmd/cronosd/dbmigrate/patch_advanced_test.go
- CHANGELOG.md
- app/storeloader.go
- cmd/cronosd/dbmigrate/migrate_dbname_test.go
- x/cronos/events/events.go
- x/cronos/types/proposal.go
- app/versiondb.go
- x/cronos/keeper/precompiles/relayer.go
- x/cronos/keeper/precompiles/bank.go
- .github/workflows/sims.yml
Previously the hoisted `mustNotInvoke` closed over the parent test's `*testing.T`. If a regression made the fast path delegate, the parent test was marked failed but the specific failing subtest wasn't, losing localization in test output. Convert `mustNotInvoke` to a factory taking the subtest `t`, so each subtest's invocation captures its own `t.Fatal` target and failure attribution is preserved.
…mment Drop the explanation of why cosmos-sdk removed NewDefaultProposalHandlerFast. Comment now states only what the function does and the contract for non-NoOp delegation.
Contributor
Author
|
Claude finished @JayT106's task in 4m 20s —— View job PR Review: feat: upgrade to cosmos-sdk v0.54.3 + ibc-go v11 + cometbft v0.39Tasks:
State-Breaking Changes 🚨This PR is intentionally state-breaking — it is a coordinated network upgrade. All validators must upgrade at the
Issues Found🔴 Critical
🟡 Important
ICA callbacks wiring — The new 🟢 Positive Changes / Observations
SummaryThe upgrade is well-structured and follows the standard Cosmos SDK major version upgrade pattern. The main open items before merging:
|
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Signed-off-by: JayT106 <JayT106@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Bot-applied suggestion (commit 00c48a3) closed the struct literal with `StoreUpgrades{}` and left the `Added: []string{}` field hanging outside it, producing two syntax errors (unexpected `}`, non-declaration outside function body). Restore the multi-line literal so the field initializer sits inside the braces.
The Added set is empty (no store-key churn from v0.53→v0.54 in this app), so wiring UpgradeStoreLoader was strictly redundant — and worse than letting the caller fall back to MaxVersionStoreLoader, which is cronos's custom loader and the loader used at every other height. Always return false so the caller installs MaxVersionStoreLoader for both regular and upgrade-height boots. Drops the upgradeInfo read, the StoreUpgrades literal, and the storetypes import.
golangci-lint thelper flagged `mustNotInvoke` (factory taking *testing.T) as a helper missing the `t.Helper()` call. Add it so failure tracebacks skip the helper frame and report the calling subtest.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
cosmossdk.io/{store,log,x/evidence,x/feegrant,x/upgrade,x/tx}standalone modules to in-treegithub.com/cosmos/cosmos-sdkpaths (store/v2, log/v2, contrib/x/crisis)PopulateQueuePendingSlotsfeat(staking): expose queue migration as utility, keep consensus version at 5 cosmos-sdk#1814fastNoOpPrepareProposal: skip per-txTxDecodefor NoOp/nil mempool; delegate to upstream default for priority mempoolscompat_mode: '0.38'+index-events: []for v0.39 ABCIProtocol Upgrade Notice
v1.8upgrade plan height.v1.8. Operators must use the matching binary; mismatched binaries will halt or fork.PopulateQueuePendingSlotsruns once at upgrade height to populate staking queue pending-slot indexes (opt-in optimization; idempotent re-runs not supported).StoreUpgrades. Both regular and upgrade-height boots use cronos'sMaxVersionStoreLoader(the existing custom loader).upgrade-info.jsonflow; no manual store ops.Relayer / Hermes Setup
nix/hermes.nix.relayer.tomlper-chain config:compat_mode = '0.38'— required so hermes treats v0.39 ABCI as Tendermint 0.38-compatible.event_source = { mode = 'push', url = 'ws://...:26657/websocket', batch_delay = '500ms' }(existing config retained).config.toml/app.toml):index-events = [](index all) — required so hermes can pull packet data; non-empty filter (e.g.['ethereum_tx.ethereumTxHash', 'message.action']) dropssend_packetevents and breaks relaying.compat_modechange; verify withhermes health-check.Follow-ups
SetRouterV2, transfer v2, attestation v2)Summary by CodeRabbit
New Features
Bug Fixes
Chores