-
Notifications
You must be signed in to change notification settings - Fork 110
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
chore: upgrade cosmos-sdk to v0.47.14 #2980
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2980 +/- ##
===========================================
- Coverage 66.09% 66.08% -0.01%
===========================================
Files 397 397
Lines 22332 22334 +2
===========================================
Hits 14760 14760
- Misses 6804 6806 +2
Partials 768 768
|
📝 WalkthroughWalkthroughThe pull request introduces updates to the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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
🧹 Outside diff range and nitpick comments (1)
go.mod (1)
Line range hint
1-371
: Consider streamlining dependency management process.The current dependency management strategy involves a complex mix of direct and indirect dependencies, replace directives, and maintained forks. To improve maintainability and reduce potential issues:
- Consider using a dependency management tool like
go mod tidy
regularly to clean up unused dependencies and ensure consistency.- Evaluate the necessity of each replace directive and maintained fork. Aim to minimize these where possible.
- Document the rationale behind each non-standard dependency management decision (e.g., why a specific fork is maintained).
- Set up automated dependency update checks (e.g., using Dependabot or similar tools) to stay informed about new versions and potential security updates.
Implementing a more streamlined dependency management process will help reduce complexity, improve maintainability, and make it easier to keep dependencies up-to-date and secure.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
go.sum
is excluded by!**/*.sum
📒 Files selected for processing (2)
- go.mod (11 hunks)
- zetaclient/context/chain.go (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
zetaclient/context/chain.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
🪛 GitHub Check: codecov/patch
zetaclient/context/chain.go
[warning] 69-71: zetaclient/context/chain.go#L69-L71
Added lines #L69 - L71 were not covered by tests
🔇 Additional comments (7)
zetaclient/context/chain.go (2)
4-4
: Import ofcmp
package is appropriate.The addition of the
cmp
package import is suitable for the subsequent changes in theAll
method. This standard library package provides efficient and type-safe comparison functions.
69-71
: Improved sorting mechanism inAll
method.The update to the sorting mechanism in the
All
method is a commendable improvement. The use ofcmp.Compare
provides a more standardized and potentially more efficient three-way comparison, which is preferable to the previous boolean-based approach.However, it is noted that these changes are not covered by unit tests. To ensure the reliability and correctness of this critical functionality, it is advisable to add appropriate test cases.
To verify the test coverage and potentially add tests, you may use the following script:
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 69-71: zetaclient/context/chain.go#L69-L71
Added lines #L69 - L71 were not covered by testsgo.mod (5)
Line range hint
1-5
: Go version and toolchain specifications are appropriate.The use of Go 1.22.2 and toolchain go1.22.5 demonstrates a commitment to leveraging the latest stable features and security updates. This is commendable for maintaining a robust and secure codebase.
Line range hint
1-371
: Summary of changes and recommendations for go.modThe go.mod file has undergone significant updates, including:
- Specification of Go version 1.22.2 and toolchain go1.22.5.
- Updates to core dependencies like cometbft, cosmos-sdk, and grpc.
- Addition of new indirect dependencies, including OpenTelemetry instrumentation.
- Modifications to replace directives and specification of ZetaChain maintained forks.
Recommendations:
- Thoroughly test the codebase with the updated dependencies, paying special attention to breaking changes.
- Monitor the impact of new indirect dependencies on performance and behavior.
- Regularly sync ZetaChain maintained forks with their upstream repositories.
- Streamline the dependency management process to reduce complexity and improve maintainability.
- Set up automated dependency update checks to stay informed about new versions and security updates.
These changes demonstrate a commitment to keeping the project up-to-date, but careful management is required to ensure long-term stability and security.
80-84
: New indirect dependencies added and existing ones updated.Several indirect dependencies have been updated, which is generally positive for bug fixes and security improvements. However, new dependencies have been introduced, notably:
- github.com/sagikazarmark/locafero v0.4.0
- github.com/sagikazarmark/slog-shim v0.1.0
- github.com/sourcegraph/conc v0.3.0
- OpenTelemetry instrumentation packages
To ensure these new dependencies don't introduce unexpected behavior or performance issues, run the following script:
#!/bin/bash # Description: Check for any potential issues with new dependencies # List all new dependencies grep -nE "require \($" go.mod -A 100 | grep -E "github.com/sagikazarmark/locafero|github.com/sagikazarmark/slog-shim|github.com/sourcegraph/conc|go.opentelemetry.io/" # Check for any known vulnerabilities go list -json -m all | go run golang.org/x/vuln/cmd/govulncheck@latestMonitor the impact of these new dependencies on build times, binary size, and runtime performance. Consider setting up benchmarks to track any changes in these metrics.
Also applies to: 88-88, 119-119, 143-143, 145-145, 153-153, 162-162, 205-205, 261-261, 263-263, 281-281, 306-308, 316-317, 323-323, 325-326, 344-345, 347-349
Line range hint
351-371
: Ensure ZetaChain maintained forks are regularly synced with upstream.The use of replace directives for ZetaChain maintained forks is noted. While this allows for customization, it's crucial to:
- Regularly sync these forks with their upstream repositories to incorporate bug fixes and security patches.
- Document the reasons for maintaining these forks and any custom changes made.
- Have a plan for eventually upstreaming these changes or migrating back to the original repositories.
To verify the current status of these forks, run the following script:
#!/bin/bash # Description: Check the status of ZetaChain maintained forks # Function to check fork status check_fork() { local fork=$1 local upstream=$2 echo "Checking $fork against $upstream" git ls-remote $fork | grep HEAD git ls-remote $upstream | grep HEAD } # Check each fork check_fork https://github.com/zeta-chain/tss-lib https://github.com/bnb-chain/tss-lib check_fork https://github.com/zeta-chain/go-ethereum https://github.com/ethereum/go-ethereum check_fork https://github.com/zeta-chain/go-libp2p https://github.com/libp2p/go-libp2p check_fork https://github.com/zeta-chain/go-tss https://gitlab.com/thorchain/tss/go-tssThis script will help identify any significant divergence between the forks and their upstream repositories.
19-23
: Significant updates to core dependencies require careful review and testing.Notable updates include:
- github.com/cometbft/cometbft v0.37.5
- github.com/cosmos/cosmos-sdk v0.47.14
- github.com/cosmos/gogoproto v1.7.0
- google.golang.org/grpc v1.62.1
These updates may introduce breaking changes or new features. Ensure that:
- The codebase has been adapted to accommodate any breaking changes.
- New features have been leveraged where appropriate.
- Comprehensive testing has been performed, including integration tests and end-to-end scenarios.
To verify the impact of these changes, run the following script:
This script will help identify any breaking changes that need to be addressed in the codebase.
Also applies to: 28-28, 32-32, 52-52, 55-55, 58-58, 66-66, 69-71
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.
Compile locally for me
0e45c22
to
57cc5d1
Compare
Description
#2934 was reverted. There doesn't seem to be any good way to get this to build on MacOS without forcing use of GCC.
Let's try only upgrading cosmos-sdk for now. That should at least unblock zeta-chain/ethermint#86 as the
x/exp
dependency was removed with the gogoproto upgrade.Closes #2959
Upgrade tests failures are unrelated: #2982
TODO:
How Has This Been Tested?
Summary by CodeRabbit
New Features
Chores