chore: remove active-precompiles-from-vm-context #1845
Conversation
randy-cro
requested review from
XinyuCRO and
calvinaco
and removed request for
a team
WalkthroughAdds an UNRELEASED changelog entry documenting removal of active precompiles from VMContext and updates Go dependency versions and gomod2nix hashes, including replaced forks for go-ethereum and ethermint. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant VMContext_before as VMContext (before)
participant VMContext_after as VMContext (after)
participant Precompile
Caller->>VMContext_before: Invoke call that may use precompile
VMContext_before->>Precompile: Check/activate precompile
Precompile-->>Caller: Return result
Caller->>VMContext_after: Invoke same call
VMContext_after-->>Caller: Handle call without active precompile check
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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. ✨ 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: 0
🧹 Nitpick comments (2)
CHANGELOG.md (2)
8-8: Fix PR number label to match the linked PR (typo).The square-bracket label shows [#1731] but links to PR 1845. Align the label with the link.
-* [#1731](https://github.com/crypto-org-chain/cronos/pull/1845) Remove active precompiles from VMContext +* [#1845](https://github.com/crypto-org-chain/cronos/pull/1845) Remove active precompiles from VMContext
5-9: Consider adding minimal migration notes for this state-machine breaking change.A one-liner on impact (e.g., behavior change around precompile activation, any param migrations, or upgrade handler expectations) will help operators plan upgrades.
📜 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 (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (2)
CHANGELOG.md(1 hunks)go.mod(3 hunks)
⏰ 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). (18)
- GitHub Check: Run golangci-lint
- GitHub Check: gomod2nix
- GitHub Check: unittest
- GitHub Check: build (ubuntu-latest)
- GitHub Check: build (macos-latest)
- GitHub Check: build (macos-14)
- GitHub Check: integration_tests (ibc_update_client)
- GitHub Check: integration_tests (ibc_rly_gas)
- GitHub Check: integration_tests (gas)
- GitHub Check: integration_tests (slow)
- GitHub Check: integration_tests (gov)
- GitHub Check: integration_tests (upgrade)
- GitHub Check: integration_tests (ica)
- GitHub Check: integration_tests (ibc_rly_evm)
- GitHub Check: integration_tests (unmarked)
- GitHub Check: integration_tests (ibc_timeout)
- GitHub Check: integration_tests (ibc)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (4)
go.mod (4)
42-42: LGTM: x/crypto patch bump.Safe, non-breaking update.
45-45: LGTM: protobuf patch bump.No concerns.
266-272: LGTM: indirect dependency patch bumps.x/net, x/sys, x/term, x/text are all minor patch bumps; low risk.
308-308: Confirmed: commit 567b9faf5406 includes the VMContext / precompiles changeCommit 567b9faf54067bd07b5ff13d01a0cd25ac5e8fcc (message: "refactor: remove unnecessary activeCompiles field in VMContext") updates VMContext and precompiles usage — e.g. it replaces:
precompiles := vm.DefaultActivePrecompiles(rules)
with:
precompiles := vm.ActivePrecompiles(cfg.Rules(big.NewInt(height), cfg.MergeNetsplitBlock != nil, timestamp))Changed files of interest:
- go.mod, go.sum
- x/evm/keeper/keeper.go
- x/evm/keeper/state_transition.go
- x/evm/keeper/state_transition_test.go
- x/evm/types/tracer.go
No further action required on the replace entry.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
|
Since you are updating the dependencies (go.mod), you also need to update the nix files with
The integration tests are using the file to setup the environment |
thomas-nguy
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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 (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (2)
go.mod(3 hunks)gomod2nix.toml(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- go.mod
⏰ 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). (20)
- GitHub Check: build (macos-14)
- GitHub Check: gomod2nix
- GitHub Check: build (macos-latest)
- GitHub Check: unittest
- GitHub Check: build (ubuntu-latest)
- GitHub Check: integration_tests (unmarked)
- GitHub Check: Run golangci-lint
- GitHub Check: integration_tests (ibc_timeout)
- GitHub Check: integration_tests (ibc_update_client)
- GitHub Check: integration_tests (ibc_rly_gas)
- GitHub Check: integration_tests (upgrade)
- GitHub Check: integration_tests (gas)
- GitHub Check: integration_tests (ibc)
- GitHub Check: integration_tests (gov)
- GitHub Check: integration_tests (ica)
- GitHub Check: integration_tests (ibc_rly_evm)
- GitHub Check: integration_tests (slow)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
gomod2nix.toml (1)
755-756: Dependency bumps in x/ and protobuf — versions in gomod2nix.toml match go.mod; please regenerate gomod2nix to confirm sha256s*Quick verification (from the provided script) — all toml entries present and either match go.mod or are indirect:
- golang.org/x/crypto — v0.41.0 — OK
- golang.org/x/net — v0.43.0 — OK (not found in go.mod; likely indirect)
- golang.org/x/sys — v0.35.0 — OK (not found in go.mod; likely indirect)
- golang.org/x/term — v0.34.0 — OK (not found in go.mod; likely indirect)
- golang.org/x/text — v0.28.0 — OK (not found in go.mod; likely indirect)
- google.golang.org/protobuf — v1.36.7 — OK
Next step (required): please rerun gomod2nix and confirm there are no nix diffs:
- Run: nix develop -c gomod2nix
- If it produces diffs, regenerate gomod2nix.toml and commit the updated file so the sha256 values are correct.
Locations to check:
- gomod2nix.toml — lines: 755-756, 761-762, 770-771, 773-774, 776-777, 797-798
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
gomod2nix.toml (1)
755-777: x/crypto, x/net, x/sys, x/term, x/text bumps mirror go.modAll versions and hashes reflect the go.mod updates. Consider adding a lightweight CI check to prevent future drift.
If desired, I can provide a small script/CI job to fail the build when gomod2nix.toml is not consistent with go.mod for selected modules.
📜 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 (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (2)
go.mod(3 hunks)gomod2nix.toml(3 hunks)
⏰ 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). (20)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: build (macos-latest)
- GitHub Check: gomod2nix
- GitHub Check: unittest
- GitHub Check: build (macos-14)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: integration_tests (slow)
- GitHub Check: integration_tests (ibc_update_client)
- GitHub Check: integration_tests (gas)
- GitHub Check: integration_tests (upgrade)
- GitHub Check: integration_tests (ica)
- GitHub Check: integration_tests (gov)
- GitHub Check: integration_tests (ibc_rly_evm)
- GitHub Check: integration_tests (ibc_timeout)
- GitHub Check: integration_tests (unmarked)
- GitHub Check: integration_tests (ibc)
- GitHub Check: integration_tests (ibc_rly_gas)
- GitHub Check: Run golangci-lint
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (7)
go.mod (4)
42-42: Bump x/crypto to v0.41.0 looks good and matches gomod2nixDependency upgrade is reasonable and is mirrored in gomod2nix.toml.
45-45: protobuf v1.36.7 bump approvedUpgrade aligns with gomod2nix and typical gRPC ecosystem compatibility.
266-271: x/net, x/sys, x/term, x/text indirect bumps are consistentAll four indirect x/* updates are reflected in gomod2nix.toml with matching versions and new hashes.
306-309: Replacements verified: go.mod/gomod2nix consistent and commit SHAs exist in forksI checked the replace entries and gomod2nix mappings and used the GitHub API to confirm the pseudo-version commits exist.
- go.mod replace block (lines ~300–340):
- github.com/ethereum/go-ethereum => github.com/crypto-org-chain/go-ethereum v1.10.20-0.20250815065500-a4fbafcae0dd
- github.com/evmos/ethermint => github.com/crypto-org-chain/ethermint v0.6.1-0.20250815070734-9e6b13895396
- gomod2nix.toml: entries for both modules have matching version and replaced fields.
- GitHub commits confirmed:
- crypto-org-chain/go-ethereum@a4fbafcae0dd — exists (HTTP 200)
- crypto-org-chain/ethermint@9e6b13895396 — exists (HTTP 200)
If you also need explicit confirmation that those commits contain the exact changes from upstream PRs (go-ethereum PR #21 and ethermint PR #691), say so and I can check the PR/commit association next.
gomod2nix.toml (3)
310-312: Updated geth fork version/hash match go.mod replacementThe pseudo-version and sha256 appear in sync with go.mod. Good pinning to a specific commit.
317-319: Updated ethermint fork version/hash match go.mod replacementAligned with go.mod and addresses previous out-of-sync feedback. Nice.
797-798: protobuf v1.36.7 hash and version update confirmedMatches go.mod; no issues.
thomas-nguy
deleted the
refactor/remove-active-precompiles-from-vm-context
branch
3 participants
main changes in:
👮🏻👮🏻👮🏻 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 👮🏻👮🏻👮🏻
PR Checklist:
make)make test)go fmt)golangci-lint run)go list -json -m all | nancy sleuth)Thank you for your code, it's appreciated! :)
Summary by CodeRabbit