feat: upgrade evm to v0.6.0 and reenable ics precompile#281
Conversation
|
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:
WalkthroughUpdates dependencies (notably cosmos/evm to v0.6.0) and Go module replacements. Replaces the local EVM IBC transfer keeper import with the IBC-Go v10 transfer keeper and re-wires transfer middleware and keeper construction. Re-enables and registers an ICS20 precompile and adds its address to the v7_1 upgrade normalization. Changes EVM keeper APIs to accept an explicit statedb and an extra boolean, updating CallEVM/CallEVMWithData call sites across wasm bindings, tests, app helpers, and precompile registration. Miscellaneous linting, Makefile, simulation API, and small test/assertion edits included. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/app_helpers.go (1)
3-32:⚠️ Potential issue | 🟡 MinorRe-run import formatting to fix CI lint failure.
The import block around Line 31 is currently failing
gci(as reported by pipeline). Please reformat this file’s imports to unblock CI.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/app_helpers.go` around lines 3 - 32, Reformat the import block in app_helpers.go to satisfy gci ordering/formatting: run your project's import formatter (e.g., goimports/gci) or IDE auto-import cleanup so standard library ("encoding/json") is grouped, third-party and internal imports are ordered per project rules and blank lines between groups are correct; ensure imports like ibckeeper, evidencekeeper, feegrantkeeper, sdk, evmkeeper, evmtypes, erc20keeper, transferkeeper, and others appear in the correct grouped order and remove any stray spacing so the file passes CI.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/upgrades/v7_1/upgrade.go`:
- Around line 38-40: Replace the hardcoded ICS20 precompile string with the
canonical constant (use the exported ics20.PrecompileAddress from the
precompiles package) and ensure insertion is idempotent by checking whether that
address already exists in newPrecompiles/ActiveStaticPrecompiles before
appending; import the precompile package, compare using the constant
(ics20.PrecompileAddress) and only append if not present to avoid duplicates.
In `@CHANGELOG.md`:
- Line 11: Update the changelog entry that currently reads "Re enabled ICS
precompile" to use the hyphenated form "Re-enabled ICS precompile" for
consistent wording; locate the exact string "Re enabled ICS precompile" in the
changelog and replace it with "Re-enabled ICS precompile".
- Line 6: Update the CHANGELOG entry to accurately reflect the actual EVM
dependency shipped: replace the plain upstream reference to v0.6.0 with the
forked module and version used in go.mod (github.com/KiiChain/evm v0.6.0-fork.1)
and mention that it's provided via a go.mod replace directive so readers can
trace the forked source.
---
Outside diff comments:
In `@app/app_helpers.go`:
- Around line 3-32: Reformat the import block in app_helpers.go to satisfy gci
ordering/formatting: run your project's import formatter (e.g., goimports/gci)
or IDE auto-import cleanup so standard library ("encoding/json") is grouped,
third-party and internal imports are ordered per project rules and blank lines
between groups are correct; ensure imports like ibckeeper, evidencekeeper,
feegrantkeeper, sdk, evmkeeper, evmtypes, erc20keeper, transferkeeper, and
others appear in the correct grouped order and remove any stray spacing so the
file passes CI.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4eee23f8-9b04-40a2-82d3-6e749f351bb5
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
CHANGELOG.mdapp/app_helpers.goapp/apptesting/evm.goapp/keepers/keepers.goapp/keepers/precompiles.goapp/upgrades/v7_1/upgrade.gogo.modwasmbinding/evm/queries.gowasmbinding/evm/reflect_query_test.go
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/keepers/precompiles.go (1)
149-156: Variable shadows package import alias.The local variable
ics20precompileon line 149 shadows the package alias imported on line 33. While this works, it's inconsistent with other precompiles in this file (e.g.,bankPrecompilevariable vsbankprecompilepackage,stakingPrecompilevsstakingprecompile).♻️ Suggested rename for consistency
- ics20precompile := ics20precompile.NewPrecompile( + ics20Precompile := ics20precompile.NewPrecompile( bankKeeper, stakingKeeper, transferKeeper, channelKeeper, erc20Keeper, )And update the registration on line 184:
- precompiles[ics20precompile.Address()] = ics20precompile + precompiles[ics20Precompile.Address()] = ics20Precompile🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/keepers/precompiles.go` around lines 149 - 156, The local variable currently named `ics20precompile` shadows the imported package alias `ics20precompile`; rename the variable to `ics20Precompile` (camelCase to match other locals like `bankPrecompile`/`stakingPrecompile`) and update any subsequent usage/registration where that variable is passed (the precompile registration call that currently references `ics20precompile`) to use `ics20Precompile` instead so the package alias remains unshadowed and naming is consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/keepers/precompiles.go`:
- Around line 149-156: The local variable currently named `ics20precompile`
shadows the imported package alias `ics20precompile`; rename the variable to
`ics20Precompile` (camelCase to match other locals like
`bankPrecompile`/`stakingPrecompile`) and update any subsequent
usage/registration where that variable is passed (the precompile registration
call that currently references `ics20precompile`) to use `ics20Precompile`
instead so the package alias remains unshadowed and naming is consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 78c2a191-991f-433b-a3ca-528307f075e6
📒 Files selected for processing (3)
app/app_helpers.goapp/keepers/keepers.goapp/keepers/precompiles.go
🚧 Files skipped from review as they are similar to previous changes (1)
- app/app_helpers.go
# Description Applies linting ## Type of change - [x] chore (Updates on dependencies, gitignore, etc)
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Makefile (1)
315-323: Centralize the golangci-lint bootstrap command.The same toolchain-prefixed install line now lives in both
lintandlint-fix, andformatbootstraps it separately again. Pulling this into one variable or helper target will make the next Go/golangci-lint bump much harder to miss.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 315 - 323, Extract the repeated bootstrap/install command used in the lint, lint-fix (and format) targets into a single reusable symbol: either a Makefile variable (e.g., GOLANGCI_BOOTSTRAP) or a phony helper target (e.g., bootstrap-golangci) and replace the inline lines in the lint and lint-fix recipes with a reference to that variable or a dependency on that helper target; update the lint, lint-fix, and format targets to use the new symbol so future Go/toolchain bumps are changed in one place (refer to the existing lint and lint-fix targets and their inline GOTOOLCHAIN=go1.24.11 go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@$(golangci_version) command when making the change).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Makefile`:
- Around line 315-323: Extract the repeated bootstrap/install command used in
the lint, lint-fix (and format) targets into a single reusable symbol: either a
Makefile variable (e.g., GOLANGCI_BOOTSTRAP) or a phony helper target (e.g.,
bootstrap-golangci) and replace the inline lines in the lint and lint-fix
recipes with a reference to that variable or a dependency on that helper target;
update the lint, lint-fix, and format targets to use the new symbol so future
Go/toolchain bumps are changed in one place (refer to the existing lint and
lint-fix targets and their inline GOTOOLCHAIN=go1.24.11 go install
github.com/golangci/golangci-lint/v2/cmd/golangci-lint@$(golangci_version)
command when making the change).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eebe9eef-0e0f-44a6-8bca-76c3960223df
📒 Files selected for processing (7)
.golangci.ymlMakefileapp/sim_test.gocmd/kiichaind/cmd/root.gox/oracle/module.gox/tokenfactory/testhelpers/authz.gox/tokenfactory/types/msgs.go
✅ Files skipped from review due to trivial changes (1)
- x/oracle/module.go
Description
Bumps EVM to v0.6.0 and re-enables ICS20 precompile
Type of change
How Has This Been Tested?
PR Checklist:
Make sure each step was done:
make lint-fix