Skip to content

feat: upgrade to cosmos-sdk v0.54.3 + ibc-go v11 + cometbft v0.39#2080

Closed
JayT106 wants to merge 49 commits into
crypto-org-chain:mainfrom
JayT106:feat/cosmos-sdk-v0.54-upgrade-wip
Closed

feat: upgrade to cosmos-sdk v0.54.3 + ibc-go v11 + cometbft v0.39#2080
JayT106 wants to merge 49 commits into
crypto-org-chain:mainfrom
JayT106:feat/cosmos-sdk-v0.54-upgrade-wip

Conversation

@JayT106
Copy link
Copy Markdown
Contributor

@JayT106 JayT106 commented May 20, 2026

Summary

  • 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)
  • v1.8 upgrade handler: opt-in PopulateQueuePendingSlots feat(staking): expose queue migration as utility, keep consensus version at 5 cosmos-sdk#1814
  • add fastNoOpPrepareProposal: skip per-tx TxDecode for NoOp/nil mempool; delegate to upstream default for priority mempools
  • wire CometBFT v0.39 app-mempool ABCI hooks behind mempool.type=app: cronos overrides ReapTxs (app/mempool/reap.go) for MaxBytes/MaxGas cap; InsertTx uses cosmos-sdk fork default (runs AnteHandler chain via RunTx(execModeCheck), maps ErrMempoolIsFullCodeTypeRetry)
  • fix ExtTxSelector gas cap: pass memTx through to parent selector so txGasLimit accumulates and maxBlockGas triggers (prevented 1B-gas blocks under app-mempool)
  • drop PriorityNonceMempool mutex during reap encode: snapshot via SelectBy under lock, encode + cap loop outside; Iterator is not thread-safe so concurrent Insert was racing the priority skiplist
  • IBC v11 wiring (gmp v2-only, ratelimit v1, IBCStackBuilder, SetICS4Wrapper)
  • hermes 1.13.1 + compat_mode: '0.38' + index-events: [] for v0.39 ABCI
  • add sharded LRU tx-decode cache: eliminates double-decode between PrepareProposal (gas accounting) and BaseApp.runTx (FinalizeBlock); installed via bApp.SetTxDecoder so all decode paths (PrepareProposal, runTx, CheckTx) share work; disable able via cronos.disable-tx-decode-cache = true in app.toml

Protocol Upgrade Notice

  • Consensus-breaking. Coordinated chain halt + binary swap at the v1.8 upgrade plan height.
  • Plan name: v1.8. Operators must use the matching binary; mismatched binaries will halt or fork.
  • State migration: PopulateQueuePendingSlots runs once at upgrade height to populate staking queue pending-slot indexes (opt-in optimization; idempotent re-runs not supported).
  • Store changes: none. No store-key churn from v0.53→v0.54 in this app, so the upgrade handler installs no StoreUpgrades. Both regular and upgrade-height boots use cronos's MaxVersionStoreLoader (the existing custom loader).
  • Cosmovisor: standard upgrade-info.json flow; no manual store ops.

Relayer / Hermes Setup

  • Bump hermes ≥ 1.13.1 (older builds reject v0.39 chain header version). Nix derivation in nix/hermes.nix.
  • relayer.toml per-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).
  • Cronos node config (config.toml / app.toml):
    • index-events = [] (index all) — required so hermes can pull packet data; non-empty filter drops send_packet events and breaks relaying.
  • No client/connection/channel rebuild required. IBC v11 is wire-compatible with v10 channels.
  • Operator action: restart hermes after compat_mode change; verify with hermes health-check.

App-mempool (mempool.type=app) operator notes

  • Default config (mempool.type=flood) unchanged; the cronos ReapTxs ABCI hook is not registered.
  • When set to app, CometBFT drives the priority mempool via ABCI:
    • InsertTx uses BaseApp's default (cosmos-sdk fork: feat(baseapp): run AnteHandler in default InsertTx handler + map ErrMempoolIsFull to CodeTypeRetry). Peer-relayed txs run the full AnteHandler chain before mempool admission. Equivalent peer-trust posture to flood.
    • ReapTxs is overridden by cronos: snapshots the priority mempool under lock, then encodes + applies RequestReapTxs.MaxBytes/MaxGas outside the lock.
  • AnteHandler now runs at admission AND at FinalizeBlock (same as flood). Per-tx admission cost ≈ RPC CheckTx cost; net throughput unchanged.
  • Misconfigured mempool.type logs Warn and falls back to flood; no panic.

Tx-decode cache (app.toml)

Key Default Effect
cronos.disable-tx-decode-cache false Set true to bypass the sharded LRU cache and use the raw decoder. Useful for debugging or profiling to isolate cache effects.

Follow-ups

  • IBC v2 routes wiring (SetRouterV2, transfer v2, attestation v2)
  • Mainnet upgrade params tuning (gov proposal)

@JayT106 JayT106 requested a review from a team as a code owner May 20, 2026 14:05
@JayT106 JayT106 requested review from calvinaco and randy-cro and removed request for a team May 20, 2026 14:05
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 20, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Coordinated 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.

Changes

Cosmos SDK v0.54 and IBC-Go v11 migration

Layer / File(s) Summary
Go toolchain and dependency manifest updates
.github/workflows/codeql-analysis.yml, .github/workflows/lint.yml, .github/workflows/sims.yml, go.mod, gomod2nix.toml, nix/build_overlay.nix, nix/sources.json
Go version bumped to 1.25.9 across CI/workflows and build tooling; go.mod and gomod2nix pins refreshed and replace directives reworked.
Systematic import path migrations to v2 modules and IBC-Go v11
app/..., cmd/..., x/cronos/..., x/e2ee/..., test/*
cosmossdk.io log/store imports moved to v2 packages and ibc-go v10→v11 applied across app, modules, precompiles, middleware, migrations, RPC, and tests.
App constructor signature and keeper field refactoring
app/app.go, cmd/cronosd/cmd/root.go
Remove traceStore io.Writer from App.New, drop commit tracer setup, and change ICA controller/host and transfer keeper fields to pointer types; update root command/appExport wiring.
Proposal selection and governance handler updates
app/proposal.go, app/proposal_test.go, app/app.go
ExtTxSelector.SelectTxForProposal drops gasWanted, validates tx bytes before delegating, removes SelectTxForProposalFast; integrate fastNoOpPrepareProposal and switch to baseapp.NewDefaultProposalHandler. Governance keeper adds default vote/votingpower calculator.
ICA callbacks and ICS4 wrapper middleware integration
app/app.go, x/cronos/middleware/conversion_middleware.go, x/cronos/middleware/conversion_middleware_test.go
Callbacks middleware constructed and configured with SetUnderlyingApplication/SetICS4Wrapper, then installed via WithICS4Wrapper; middleware implements SetICS4Wrapper to satisfy v11 IBCModule contract; tests updated.
Upgrade handler and store loader logic
app/upgrades.go
Upgrade handler now populates staking queue slots before migrations; post-registration reads upgrade info from disk and conditionally sets UpgradeStoreLoader when stored plan matches local plan and height not skipped.
Node service, block-stm runner, and supporting app updates
app/app.go
RegisterNodeService supplies earliest-version callback; RefreshBlockList call annotated with //nolint:staticcheck; STM runner uses txnrunner.NewSTMRunner; legacy IBC param key-table registrations removed.
Test and test-helper app initialization updates
app/bench_test.go, app/test_helpers.go, app/sim_test.go, testutil/simapp/simapp.go, x/cronos/keeper/keeper_test.go
Update app.New(...) callsites to new constructor form (replace nil with boolean), and adapt keeper test helpers (e.g., NextAccountNumber usage).
Telemetry lint annotations, migration codec, and minor fixes
x/cronos/keeper/ibc.go, x/cronos/types/proposal.go, x/e2ee/keyring/keyring.go, x/cronos/migrations/v2/migrate_test.go, cmd/cronosd/dbmigrate/patch.go
Add //nolint:staticcheck on telemetry calls, simplify String() formatting, fix keyring error sentinel, and switch test codec to ethermint encoding where applicable.
Integration test Hermes compatibility and changelog
integration_tests/configs/ibc.jsonnet, CHANGELOG.md
Adjust index-events and add compat_mode: '0.38' for Hermes; add UNRELEASED changelog entry documenting SDK/IBC/CometBFT upgrades.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • calvinaco
  • randy-cro

🐰 I hopped through imports and bumped the Go,
From v10 to v11 the IBC seeds flow,
Keepers turned pointers, middleware set free,
Tests and builds updated—one-two-three,
Cheers! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly and specifically summarizes the main objective: upgrading to three major dependencies (cosmos-sdk v0.54.3, ibc-go v11, cometbft v0.39), which aligns perfectly with the extensive changeset that updates imports, wiring, and module APIs across the codebase.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

This comment has been minimized.

Comment thread go.mod Outdated
@socket-security
Copy link
Copy Markdown

socket-security Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
docs/architecture/cosmos-sdk-v0.54-upgrade-plan.md (1)

22-45: ⚡ Quick win

Avoid forcing persistent global git rewrites in the default setup path.

Using git config --global ... insteadOf can 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

📥 Commits

Reviewing files that changed from the base of the PR and between 919769c and 23dd931.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (48)
  • .github/workflows/codeql-analysis.yml
  • .github/workflows/lint.yml
  • .github/workflows/sims.yml
  • app/app.go
  • app/bench_test.go
  • app/export.go
  • app/proposal.go
  • app/proposal_test.go
  • app/sim_test.go
  • app/storeloader.go
  • app/test_helpers.go
  • app/upgrades.go
  • app/versiondb.go
  • app/versiondb_placeholder.go
  • cmd/cronosd/cmd/root.go
  • cmd/cronosd/dbmigrate/migrate.go
  • cmd/cronosd/dbmigrate/migrate_basic_test.go
  • cmd/cronosd/dbmigrate/migrate_dbname_test.go
  • cmd/cronosd/dbmigrate/migrate_enhanced_test.go
  • cmd/cronosd/dbmigrate/migrate_rocksdb_test.go
  • cmd/cronosd/dbmigrate/migrate_test.go
  • cmd/cronosd/dbmigrate/patch.go
  • cmd/cronosd/dbmigrate/patch_advanced_test.go
  • cmd/cronosd/dbmigrate/patch_integration_test.go
  • docs/architecture/cosmos-sdk-v0.54-upgrade-plan.md
  • go.mod
  • gomod2nix.toml
  • testutil/simapp/simapp.go
  • x/cronos/client/cli/tx.go
  • x/cronos/events/decoders.go
  • x/cronos/events/events.go
  • x/cronos/keeper/ibc.go
  • x/cronos/keeper/keeper.go
  • x/cronos/keeper/keeper_test.go
  • x/cronos/keeper/mock/ibckeeper_mock.go
  • x/cronos/keeper/params.go
  • x/cronos/keeper/precompiles/bank.go
  • x/cronos/keeper/precompiles/ica.go
  • x/cronos/keeper/precompiles/relayer.go
  • x/cronos/middleware/conversion_middleware.go
  • x/cronos/middleware/conversion_middleware_test.go
  • x/cronos/migrations/v2/migrate.go
  • x/cronos/migrations/v2/migrate_test.go
  • x/cronos/rpc/api.go
  • x/cronos/types/interfaces.go
  • x/cronos/types/proposal.go
  • x/e2ee/keeper/keeper.go
  • x/e2ee/keyring/keyring.go
💤 Files with no reviewable changes (1)
  • app/proposal_test.go

Comment thread docs/architecture/cosmos-sdk-v0.54-upgrade-plan.md Outdated
Comment thread docs/architecture/cosmos-sdk-v0.54-upgrade-plan.md Outdated
Comment thread go.mod Outdated
@JayT106 JayT106 force-pushed the feat/cosmos-sdk-v0.54-upgrade-wip branch 2 times, most recently from efa739a to 9374171 Compare May 20, 2026 14:56
@JayT106 JayT106 marked this pull request as draft May 20, 2026 14:57
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 23dd931 and 2c7cfe7.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (48)
  • .github/workflows/codeql-analysis.yml
  • .github/workflows/lint.yml
  • .github/workflows/sims.yml
  • app/app.go
  • app/bench_test.go
  • app/export.go
  • app/proposal.go
  • app/proposal_test.go
  • app/sim_test.go
  • app/storeloader.go
  • app/test_helpers.go
  • app/upgrades.go
  • app/versiondb.go
  • app/versiondb_placeholder.go
  • cmd/cronosd/cmd/root.go
  • cmd/cronosd/dbmigrate/migrate.go
  • cmd/cronosd/dbmigrate/migrate_basic_test.go
  • cmd/cronosd/dbmigrate/migrate_dbname_test.go
  • cmd/cronosd/dbmigrate/migrate_enhanced_test.go
  • cmd/cronosd/dbmigrate/migrate_rocksdb_test.go
  • cmd/cronosd/dbmigrate/migrate_test.go
  • cmd/cronosd/dbmigrate/patch.go
  • cmd/cronosd/dbmigrate/patch_advanced_test.go
  • cmd/cronosd/dbmigrate/patch_integration_test.go
  • docs/architecture/cosmos-sdk-v0.54-upgrade-plan.md
  • go.mod
  • gomod2nix.toml
  • testutil/simapp/simapp.go
  • x/cronos/client/cli/tx.go
  • x/cronos/events/decoders.go
  • x/cronos/events/events.go
  • x/cronos/keeper/ibc.go
  • x/cronos/keeper/keeper.go
  • x/cronos/keeper/keeper_test.go
  • x/cronos/keeper/mock/ibckeeper_mock.go
  • x/cronos/keeper/params.go
  • x/cronos/keeper/precompiles/bank.go
  • x/cronos/keeper/precompiles/ica.go
  • x/cronos/keeper/precompiles/relayer.go
  • x/cronos/middleware/conversion_middleware.go
  • x/cronos/middleware/conversion_middleware_test.go
  • x/cronos/migrations/v2/migrate.go
  • x/cronos/migrations/v2/migrate_test.go
  • x/cronos/rpc/api.go
  • x/cronos/types/interfaces.go
  • x/cronos/types/proposal.go
  • x/e2ee/keeper/keeper.go
  • x/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

Comment thread app/upgrades.go Outdated
Comment thread gomod2nix.toml Outdated
@JayT106 JayT106 force-pushed the feat/cosmos-sdk-v0.54-upgrade-wip branch from d7524ed to 79983fa Compare May 20, 2026 15:27
@github-actions github-actions Bot added the build label May 20, 2026
@JayT106 JayT106 force-pushed the feat/cosmos-sdk-v0.54-upgrade-wip branch 3 times, most recently from 6a2b173 to e8437d0 Compare May 20, 2026 15:56
@github-actions github-actions Bot added the nix label May 20, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 20, 2026

Codecov Report

❌ Patch coverage is 79.41176% with 42 lines in your changes missing coverage. Please review.
✅ Project coverage is 15.58%. Comparing base (a3c2f70) to head (3e3198a).
⚠️ Report is 201 commits behind head on main.

Files with missing lines Patch % Lines
x/cronos/middleware/conversion_middleware.go 50.00% 10 Missing and 3 partials ⚠️
app/decode_cache.go 79.24% 8 Missing and 3 partials ⚠️
app/upgrades.go 0.00% 6 Missing ⚠️
app/mempool/reap.go 89.65% 2 Missing and 1 partial ⚠️
x/cronos/keeper/precompiles/ica.go 0.00% 3 Missing ⚠️
app/mempool/insert.go 95.12% 2 Missing ⚠️
x/cronos/types/proposal.go 0.00% 2 Missing ⚠️
testutil/simapp/simapp.go 0.00% 1 Missing ⚠️
x/e2ee/keyring/keyring.go 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2080      +/-   ##
==========================================
- Coverage   16.87%   15.58%   -1.30%     
==========================================
  Files          72       77       +5     
  Lines        6163     5249     -914     
==========================================
- Hits         1040      818     -222     
+ Misses       5000     4351     -649     
+ Partials      123       80      -43     
Files with missing lines Coverage Δ
app/export.go 0.00% <ø> (ø)
app/proposal.go 34.32% <100.00%> (+15.90%) ⬆️
app/storeloader.go 0.00% <ø> (ø)
app/versiondb_placeholder.go 0.00% <ø> (ø)
x/cronos/client/cli/tx.go 0.00% <ø> (-38.88%) ⬇️
x/cronos/events/decoders.go 0.00% <ø> (ø)
x/cronos/events/events.go 0.00% <ø> (-23.08%) ⬇️
x/cronos/keeper/ibc.go 83.48% <100.00%> (+83.48%) ⬆️
x/cronos/keeper/keeper.go 68.59% <ø> (+61.52%) ⬆️
x/cronos/keeper/mock/ibckeeper_mock.go 0.00% <ø> (ø)
... and 15 more

... and 52 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JayT106 JayT106 force-pushed the feat/cosmos-sdk-v0.54-upgrade-wip branch 5 times, most recently from 0063035 to adf8c57 Compare May 21, 2026 01:29
JayT106 added 2 commits May 28, 2026 17:48
- app/proposal: cap NoOp PrepareProposal selected gas at consensus MaxGas
  (decode-on-demand) so proposals do not exceed the block gas budget.
  Document that non-FeeTx is admitted without gas accounting — unreachable
  in practice since all cronos txs implement sdk.FeeTx.
- middleware/conversion: only commit cacheCtx when the transfer ack succeeds,
  preventing persisted cacheCtx writes when the underlying transfer fails.
- precompiles/ica: early-return baseCost on unknown method ID before checking
  SubmitMsgsMethodName, making the control flow explicit.
- app/upgrades: clarify v1.8 PopulateQueuePendingSlots is idempotent
  (keeper Set overwrites the per-time slot).
- app/mempool/reap_test: add post-quiesce double-reap count-stability check
  to the concurrent insert race test; reference cosmos/cosmos-sdk#1751 for
  the upstream per-tx uniqueness issue deferred from this scope.
- Makefile: add test-race-mempool target; CI: add race-detector mempool step.
- integration_tests/test_app_mempool.py: collapse isort import to one line.
@JayT106 JayT106 force-pushed the feat/cosmos-sdk-v0.54-upgrade-wip branch from 8d6bdec to 0c2fa8a Compare May 28, 2026 23:27
JayT106 added 2 commits May 28, 2026 19:33
Ethermint @ eb38211d (jt/feat-sdk-54) builds against
crypto-org-chain/go-ethereum release/v1.16 which adds SkipTransactionChecks
to core.Message. The branch was accidentally pinned to release/v1.15
(a4fbafcae0dd) after the rebase-main merge resolved the replace directive
conflict on the wrong side.

Update go.mod replace directive to the release/v1.16 pseudoversion and
refresh gomod2nix.toml hashes accordingly.
PrepareProposal decodes each tx to read gas (when MaxBlockGas > 0);
BaseApp.runTx decodes the same bytes again during FinalizeBlock. Install
a shared caching decoder via bApp.SetTxDecoder so both paths share work.

Implementation:
- 16 shards × 625-entry LRU (container/list + per-shard sync.Mutex)
- LRU eviction keeps re-proposed txs warm across block rounds
- Per-entry 64KB size cap bounds worst-case memory to ~640MB
- xxhash routing with byte-level collision guard
- cachingDecoder also installed on app.txDecoder (CheckTx path)

Verified: go test -race ./app/... passes; 7 focused cache tests cover
hit/miss, error-not-cached, LRU order, eviction bound, concurrency,
and size-capped payloads.
@songgaoye
Copy link
Copy Markdown
Contributor

as follow up we can override InsertTx to improve performance. right?

@JayT106 JayT106 force-pushed the feat/cosmos-sdk-v0.54-upgrade-wip branch from 0c2fa8a to 3aca3d1 Compare May 29, 2026 00:50
@JayT106
Copy link
Copy Markdown
Contributor Author

JayT106 commented May 29, 2026

as follow up we can override InsertTx to improve performance. right?

I think so. I added some cache layer for decoder in this PR.

JayT106 added 3 commits May 28, 2026 21:47
… DoS

Without a seed, xxhash.Sum64 is deterministic and public — an attacker can
precompute which tx bytes land in which shard and flood a single shard with
625 crafted (valid, signed) txs, evicting all real txs and spiking cache
miss rate for 1/16 of traffic.

XOR with a per-process random seed at startup makes shard assignment
unpredictable. Preserves xxhash distribution (XOR with constant is a bijection).
Correctness is unaffected: bytes.Equal guards against both collisions and
cross-shard lookups.
Add cronos.disable-tx-decode-cache app.toml flag. When true, the raw
TxDecoder is used directly instead of the sharded LRU caching decoder.
Useful for debugging or profiling to isolate cache effects.

Default (unset / false) keeps the cache active.
Comment thread app/upgrades.go Outdated
Comment thread app/mempool/reap.go
Comment thread app/app.go
Comment thread app/proposal.go
@thomas-nguy
Copy link
Copy Markdown
Collaborator

thomas-nguy commented May 29, 2026

Overall looks fine but can you change the PR description to split

  • What is purely part of cosmos sdk v54 integration

  • What is new optimisations or new features?

This PR seems to do too many things, for reference
crypto-org-chain/ethermint#894

Comment thread app/app.go Outdated
Comment thread app/proposal.go Outdated
Comment thread app/upgrades.go
JayT106 added 5 commits May 29, 2026 10:54
Pin cosmos-sdk fork to pre-PR-1815 commit (7d79ddc38152), reverting the
SDK-default InsertTx AnteHandler path. Implement equivalent validation in
cronos itself via NewInsertTxHandler in app/mempool/insert.go.

- Runs RunTx(ExecModeCheck) on each peer-relayed tx before admission,
  mirroring CheckTx semantics: bad-sig/replay/wrong-chainID rejected at
  insertion, not at FinalizeBlock.
- FIFO seen-cache (default 16384, tunable via mempool.insert-tx-cache-size)
  deduplicates AnteHandler cost on gossip fan-out (~82ns cache-hit vs
  ~500µs RunTx).
- ErrMempoolTxMaxCapacity maps to CodeTypeRetry so peers backoff gracefully.
- txRunner interface keeps BaseApp dependency injectable for unit tests.
- upgrades.go: extract planName const instead of inline "v1.8" literal
- proposal.go: fix uint64 overflow in gas accumulation (gasWanted > maxBlockGas-totalGas)
- app.go: panic on unrecognized mempool.type instead of silent warn
- decode_cache.go: fix misspelling randomises→randomizes
- mempool/insert{,_test}.go: fix gci import ordering
Previous test asserted parent.lastMemTx==nil (old buggy behavior).
Replace with:
- assertion that real memTx reaches parent unmodified
- gas-cap integration test: gasCapSelector rejects second tx when
  first already consumed 60k of 100k budget, proving the nil-was-bug fix
Defensive ordering: build pending-slot indexes on fully-migrated
queue keys so a future staking migration that rewrites ValidatorQueueKey
or UnbondingQueueKey doesn't leave the indexes stale at upgrade height.
@JayT106
Copy link
Copy Markdown
Contributor Author

JayT106 commented May 29, 2026

Splitting this PR per @thomas-nguy's request in #issuecomment-4572434133.

Closing this PR.

@JayT106 JayT106 closed this May 29, 2026
JayT106 added a commit that referenced this pull request May 29, 2026
- fix(exchange): add Name() to ExchangeContract to satisfy vm.PrecompiledContract
- fix(upgrades): move PopulateQueuePendingSlots after RunMigrations so indexes
  are built on fully-migrated queue keys
- test(proposal): add test proving ExtTxSelector validates with original memTx
  but forwards nil to parent to bypass block-gas enforcement
- docs(middleware): clarify IBCConversionModule only implements IBCModule, not
  full porttypes.Middleware (missing SetUnderlyingApplication)
JayT106 added a commit that referenced this pull request May 29, 2026
- upgrades.go: extract planName const instead of inline "v1.8" literal
- proposal.go: fix uint64 overflow in gas accumulation (gasWanted > maxBlockGas-totalGas)
- app.go: panic on unrecognized mempool.type instead of silent warn
- decode_cache.go: fix misspelling randomises→randomizes
- mempool/insert{,_test}.go: fix gci import ordering
JayT106 added a commit that referenced this pull request May 29, 2026
- upgrades.go: extract planName const instead of inline "v1.8" literal
- proposal.go: fix uint64 overflow in gas accumulation (gasWanted > maxBlockGas-totalGas)
- app.go: panic on unrecognized mempool.type instead of silent warn
- decode_cache.go: fix misspelling randomises→randomizes
- mempool/insert{,_test}.go: fix gci import ordering
JayT106 added a commit that referenced this pull request May 29, 2026
- upgrades.go: extract planName const instead of inline "v1.8" literal
- proposal.go: fix uint64 overflow in gas accumulation (gasWanted > maxBlockGas-totalGas)
- app.go: panic on unrecognized mempool.type instead of silent warn
- decode_cache.go: fix misspelling randomises→randomizes
- mempool/insert{,_test}.go: fix gci import ordering
JayT106 added a commit that referenced this pull request May 29, 2026
- upgrades.go: extract planName const instead of inline "v1.8" literal
- proposal.go: fix uint64 overflow in gas accumulation (gasWanted > maxBlockGas-totalGas)
- app.go: panic on unrecognized mempool.type instead of silent warn
- decode_cache.go: fix misspelling randomises→randomizes
- mempool/insert{,_test}.go: fix gci import ordering
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants