-
Notifications
You must be signed in to change notification settings - Fork 276
feat: Enforce app creator returning application implement AppWithPendingTxStream during compilation phase #1846
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds an UNRELEASED changelog entry documenting PR #1846 about enforcing app creator return type; updates dependency versions and hashes for several modules (notably Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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. ✨ 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 (
|
…ingTxStream during compilation phase
a0a9481 to
e67be90
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CHANGELOG.md (1)
9-9: Grammar fix for clarity.Minor wording nit; “application implement” reads awkwardly.
Apply this diff to improve readability:
-* [#1846](https://github.com/crypto-org-chain/cronos/pull/1846) Enforce app creator returning application implement AppWithPendingTxStream during compilation phase +* [#1846](https://github.com/crypto-org-chain/cronos/pull/1846) Enforce app creator to return an application implementing AppWithPendingTxStream at compile time
📜 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 (3)
CHANGELOG.md(1 hunks)go.mod(1 hunks)gomod2nix.toml(1 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: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: build (macos-14)
- GitHub Check: gomod2nix
- GitHub Check: integration_tests (gov)
- GitHub Check: integration_tests (ibc_rly_evm)
- GitHub Check: integration_tests (upgrade)
- GitHub Check: integration_tests (gas)
- GitHub Check: integration_tests (ibc_update_client)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: build (macos-latest)
- GitHub Check: Run golangci-lint
- GitHub Check: integration_tests (ibc_timeout)
- GitHub Check: integration_tests (unmarked)
- GitHub Check: integration_tests (ica)
- GitHub Check: unittest
- GitHub Check: integration_tests (slow)
- GitHub Check: integration_tests (ibc_rly_gas)
- GitHub Check: integration_tests (ibc)
🔇 Additional comments (2)
go.mod (1)
308-308: Bump to Ethermint commit looks correct — gomod2nix aligned; please run CI/buildChecked alignment between go.mod and gomod2nix.toml — versions match and a hash is present.
- go.mod (line ~308): github.com/evmos/ethermint => github.com/crypto-org-chain/ethermint v0.6.1-0.20250815090753-0c0f827db994
- gomod2nix.toml (mod."github.com/evmos/ethermint"): version = "v0.6.1-0.20250815090753-0c0f827db994", hash = "sha256-kdRYx+HUVgI2rqEz5ulI0Jj/VQ2sjwoXkjq2TsjmXWY=", replaced = "github.com/crypto-org-chain/ethermint"
Result: no go.mod/gomod2nix drift found. Recommend running the project's CI/full build to confirm there are no compile-time breaks (e.g., AppWithPendingTxStream enforcement).
gomod2nix.toml (1)
317-319: gomod2nix pin and hash updated in sync with go.mod.The Ethermint version and sha256 were updated and the replaced field remains the Cronos fork. This looks aligned with the go.mod replace bump.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
go.mod (2)
84-85: Sonic and loader upgrades: validate cross-arch buildsUpgrading github.com/bytedance/sonic to v1.14.0 and loader to v0.3.0 is fine, but sonic uses SIMD/assembly paths. Thanks for also bumping golang.org/x/arch. Please ensure CI covers linux/amd64, linux/arm64, darwin/amd64, and darwin/arm64 to catch any arch-specific build issues.
308-315: ethermint replace bumped to a commit with pending-tx stream enforcement: sync checkPointing github.com/evmos/ethermint to github.com/crypto-org-chain/ethermint v0.6.1-0.20250818034006-1a6e81479555 aligns with the PR goal. Two asks:
- Verify this exact commit contains the compile-time enforcement for AppWithPendingTxStream.
- Once upstream tags a release, consider switching from a pseudo-version to a tag for stability and reproducibility.
Optional: add an explicit compile-time assertion in Cronos app code to make the requirement obvious locally (in the app package):
// Ensure our app implements the required interface from Ethermint server. var _ server.AppWithPendingTxStream = (*App)(nil)gomod2nix.toml (1)
151-156: Sonic and loader nix pins updated; ensure generated via gomod2nixVersions and hashes reflect the upgrades. If not already, regenerate via gomod2nix to avoid drift.
📜 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(4 hunks)gomod2nix.toml(4 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: build (ubuntu-latest)
- GitHub Check: gomod2nix
- GitHub Check: build (macos-14)
- GitHub Check: contracts
- GitHub Check: unittest
- GitHub Check: Run golangci-lint
- GitHub Check: integration_tests (gas)
- GitHub Check: integration_tests (gov)
- GitHub Check: integration_tests (slow)
- GitHub Check: integration_tests (ibc)
- GitHub Check: integration_tests (ica)
- GitHub Check: integration_tests (ibc_rly_gas)
- GitHub Check: integration_tests (upgrade)
- GitHub Check: integration_tests (ibc_timeout)
- GitHub Check: integration_tests (unmarked)
- GitHub Check: integration_tests (ibc_rly_evm)
- GitHub Check: integration_tests (ibc_update_client)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (5)
go.mod (2)
10-10: Minor bump to cosmossdk.io/log looks safev1.6.1 is a minor update; no breaking API expected. Good to keep logging stack current.
264-264: golang.org/x/arch to v0.17.0 aligns with sonic updateMatches the SIMD/assembly ecosystem versions. No concerns.
gomod2nix.toml (3)
47-48: cosmossdk.io/log nix pin updated with matching hashVersion and sha256 updated together; looks consistent with go.mod.
317-319: ethermint nix pin aligns with replace; goodThe module is still replaced to github.com/crypto-org-chain/ethermint and the pseudo-version/hash is updated. This keeps Nix build reproducible with the enforced interface upstream.
752-753: golang.org/x/arch nix pin bumped with matching hashIn sync with go.mod; no issues.
Description
Changes in ethermint: crypto-org-chain/ethermint#692
👮🏻👮🏻👮🏻 !!!! 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
Documentation
Chores