Skip to content
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 cometbft to v0.37.12 #3045

Merged
merged 3 commits into from
Oct 29, 2024
Merged

Conversation

gartnera
Copy link
Member

@gartnera gartnera commented Oct 25, 2024

Description

cometbft/cometbft#4294 was released in v0.37.12 which means that we can upgrade cometbft now.

This sidesteps all the issues described in #2934 and #2959

Update: should upgrade gosec first to avoid those warnings #2933

Upgrade test failures appear unrelated, removing label:

Error Trace:	/go/delivery/zeta-node/e2e/utils/require.go:22
	            	/go/delivery/zeta-node/e2e/e2etests/test_bitcoin_std_deposit.go:51
	Error:      	Not equal: 
	            	expected: 3
	            	actual  : 5
	Messages:   	cctx status is not "OutboundMined" cctx index 0x9f4c88aad8491c8d00ecb4b5cf228231920444891ecf8f1e[149](https://github.com/zeta-chain/node/actions/runs/11563579270/job/32187162790?pr=3045#step:13:150)75ac6d98283f0

How Has This Been Tested?

  • Tested CCTX in localnet
  • Tested in development environment
  • Go unit tests
  • Go integration tests
  • Tested via GitHub Actions

Summary by CodeRabbit

  • New Features

    • Upgraded the Go version used in the application to enhance performance and compatibility.
    • Updated dependencies to their latest versions for improved functionality and security.
  • Bug Fixes

    • Resolved potential issues by updating several key libraries and tools.
  • Chores

    • Updated Dockerfiles and Makefile to reflect new Go versions, ensuring consistency across build environments.

@gartnera gartnera added no-changelog Skip changelog CI check UPGRADE_TESTS Run make start-upgrade-tests UPGRADE_IMPORT_MAINNET_TESTS Run make start-upgrade-import-mainnet-test PERFORMANCE_TESTS Run make start-e2e-performance-test labels Oct 25, 2024
Copy link
Contributor

coderabbitai bot commented Oct 25, 2024

📝 Walkthrough

Walkthrough

The pull request includes updates to several files, primarily focusing on upgrading the Golang version used in various Dockerfiles and the go.mod file. The Dockerfile-localnet and contrib/localnet/orchestrator/Dockerfile.fastbuild have been modified to use Golang version 1.22.7, while the Makefile reflects an update to the GOLANG_CROSS_VERSION. Additionally, the go.mod file has been revised to update the Go version and several dependencies, ensuring compatibility with the latest features and fixes.

Changes

File Change Summary
Dockerfile-localnet Updated base image from ghcr.io/zeta-chain/golang:1.22.5-bookworm to ghcr.io/zeta-chain/golang:1.22.7-bookworm across multiple stages.
contrib/localnet/orchestrator/Dockerfile.fastbuild Updated base image from ghcr.io/zeta-chain/golang:1.22.5-bookworm to ghcr.io/zeta-chain/golang:1.22.7-bookworm.
Makefile Updated GOLANG_CROSS_VERSION from v1.22.4 to v1.22.7.
go.mod Updated Go version from 1.22.2 to 1.22.7, toolchain from go1.22.5 to go1.22.8, and modified several dependencies.

Possibly related PRs

  • fix: update go-tss with known fixes #2847: The go.mod file was updated to reflect changes in Go version and dependencies, which is related to the main PR's update of the Golang base image in the Dockerfile.
  • chore: bump ethermint #2854: Similar to the main PR, this PR updates the go.mod file to modify version requirements for dependencies, indicating a connection through the management of Go versions.
  • chore: improve localnet build performance #2928: This PR modifies the Dockerfile-localnet, which is directly related to the main PR's changes in the same file, specifically regarding the base image and build stages.
  • chore: always set explicit base compose file #3037: Changes in the Makefile regarding Docker Compose commands may indirectly relate to the Dockerfile changes in the main PR, as they both involve the build process and configuration for localnet testing.

Suggested labels

UPGRADE_TESTS, UPGRADE_IMPORT_MAINNET_TESTS

Suggested reviewers

  • fbac
  • kingpinXD
  • ws4charlie
  • brewmaster012
  • skosito
  • lumtis

📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between d9ee4f3 and 0e8f2c0.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (4)
  • Dockerfile-localnet (2 hunks)
  • Makefile (1 hunks)
  • contrib/localnet/orchestrator/Dockerfile.fastbuild (1 hunks)
  • go.mod (14 hunks)
✅ Files skipped from review due to trivial changes (2)
  • Makefile
  • contrib/localnet/orchestrator/Dockerfile.fastbuild
🔇 Additional comments (6)
Dockerfile-localnet (2)

3-3: LGTM: Go version upgrade is consistent across all stages.

The upgrade from Go 1.22.5 to 1.22.7 is properly applied across all build stages, maintaining consistency.

Let's verify the Go version compatibility with cometbft v0.37.12:

Also applies to: 30-30, 33-33

✅ Verification successful

Go version upgrade is compatible with cometbft v0.37.12

The upgrade to Go 1.22.7 aligns with the cometbft v0.37.12 dependency requirements. The version is consistently declared in both go.mod and Dockerfile-localnet, maintaining build reproducibility across environments.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify Go version compatibility with cometbft dependency

# Test: Check go.mod for cometbft version and Go requirements
rg -A 5 "github.com/cometbft/cometbft" go.mod

Length of output: 82204


Script:

#!/bin/bash
# Check go.mod for cometbft version and Go requirements
rg -p "cometbft/cometbft.*v0.37.12" go.mod

# Check if Go version is compatible with cometbft v0.37.12
rg -p "^go " go.mod

Length of output: 120


Line range hint 1-100: Verify build reproducibility after Go version upgrade.

Since this is a build environment change, it's important to ensure build reproducibility.

Run the following script to check for any version-specific build flags or constraints:

✅ Verification successful

Go version upgrade is safe to proceed

The codebase shows good compatibility with the Go version upgrade:

  • Only one version-specific constraint exists in debug package for Go 1.5, which is well below our target version
  • No other version-specific build tags or constraints that would be impacted
  • The toolchain is already set to go1.22.8 in go.mod, aligning with the upgrade
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for Go version constraints in build files

# Test 1: Look for Go version constraints in build files
rg -g '*.{go,mod,sum,mk,Makefile}' -A 3 '(go |GO_VERSION|GOLANG_VERSION|go1\.[0-9]+)'

# Test 2: Check for any Go build tags or constraints
fd -e go -x grep -l '^//go:build' {} \;

Length of output: 82204


Script:

#!/bin/bash
# Check for Go version-specific build tags and constraints in the codebase
# Look for any build tags or constraints that might be affected by Go version upgrade

# Check for Go version constraints in build files and Go files
rg -g '*.{go,mod,sum,mk,Makefile}' -A 3 'go1\.[0-9]+' --no-filename

# Check for build tags that might be version-specific
rg -g '*.go' '^//\+build go[0-9]' --no-filename

# Check for any Go build constraints that might affect compilation
rg -g '*.go' '^//go:build.*go[0-9]' --no-filename

# Look for any Go version-specific code or features
rg -g '*.go' 'runtime\.Version|go1\.[0-9]+' --no-filename

Length of output: 562

go.mod (4)

3-5: LGTM: Go version and toolchain updates.

The upgrade to Go 1.22.7 and toolchain 1.22.8 keeps the project current with the latest stable releases while maintaining backward compatibility within the 1.22.x series.


Line range hint 365-377: LGTM: Repository replacements are well-documented.

The replace directives are properly documented with comments explaining the reasons for the replacements and linking to the relevant forks and issues.


339-343: Verify new indirect dependencies.

New indirect dependencies have been added:

  • github.com/cockroachdb/fifo
  • github.com/hashicorp/golang-lru/v2
  • github.com/kylelemons/godebug
  • github.com/munnerz/goautoneg

Let's verify these are actually required and not accidentally included.

✅ Verification successful

Indirect dependencies verification complete

The dependencies are legitimately required:

  • github.com/hashicorp/golang-lru/v2 is actively used across multiple files in the zetaclient for LRU caching functionality
  • Other indirect dependencies (cockroachdb/fifo, kylelemons/godebug, munnerz/goautoneg) are transitive dependencies pulled in by direct dependencies, which is normal Go module behavior
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for usage of new indirect dependencies
for dep in "cockroachdb/fifo" "hashicorp/golang-lru/v2" "kylelemons/godebug" "munnerz/goautoneg"; do
    echo "Checking usage of $dep:"
    rg "\"github.com/$dep\""
done

Length of output: 897


Script:

#!/bin/bash
# Let's try a different approach by checking go.mod history and dependencies
# First, let's see what changed in go.mod
git diff HEAD^ HEAD go.mod

# Then let's check if these are transitive dependencies
go mod why github.com/cockroachdb/fifo
go mod why github.com/hashicorp/golang-lru/v2
go mod why github.com/kylelemons/godebug
go mod why github.com/munnerz/goautoneg

# Also check for any imports that might be using these packages
# Using -l to include only file names to check if these are used anywhere
rg -l "cockroachdb/fifo|hashicorp/golang-lru|kylelemons/godebug|munnerz/goautoneg"

Length of output: 847


Script:

#!/bin/bash
# Let's examine the actual usage in the files found
echo "=== Checking zetaclient/chains/bitcoin/observer/observer_test.go ==="
rg -C 2 "cockroachdb/fifo|hashicorp/golang-lru|kylelemons/godebug|munnerz/goautoneg" zetaclient/chains/bitcoin/observer/observer_test.go

echo "=== Checking zetaclient/chains/base/observer.go ==="
rg -C 2 "cockroachdb/fifo|hashicorp/golang-lru|kylelemons/godebug|munnerz/goautoneg" zetaclient/chains/base/observer.go

echo "=== Checking zetaclient/chains/base/observer_test.go ==="
rg -C 2 "cockroachdb/fifo|hashicorp/golang-lru|kylelemons/godebug|munnerz/goautoneg" zetaclient/chains/base/observer_test.go

echo "=== Checking zetaclient/chains/evm/observer/outbound_test.go ==="
rg -C 2 "cockroachdb/fifo|hashicorp/golang-lru|kylelemons/godebug|munnerz/goautoneg" zetaclient/chains/evm/observer/outbound_test.go

echo "=== Checking zetaclient/chains/evm/observer/observer_test.go ==="
rg -C 2 "cockroachdb/fifo|hashicorp/golang-lru|kylelemons/godebug|munnerz/goautoneg" zetaclient/chains/evm/observer/observer_test.go

echo "=== Checking zetaclient/chains/ton/liteapi/client.go ==="
rg -C 2 "cockroachdb/fifo|hashicorp/golang-lru|kylelemons/godebug|munnerz/goautoneg" zetaclient/chains/ton/liteapi/client.go

Length of output: 2405


19-20: Verify cometbft upgrade impact.

The upgrade of github.com/cometbft/cometbft to v0.37.12 and its DB component to v0.14.1 aligns with the PR objectives. However, let's verify there are no breaking changes in the upgrade path.

✅ Verification successful

Let me analyze the codebase usage of CometBFT to verify the upgrade impact.


CometBFT upgrade from v0.37.4 to v0.37.12 appears safe to proceed

Based on the comprehensive analysis of the codebase:

  • The upgrade is within the same minor version (0.37.x), indicating backward compatibility
  • No breaking changes are detected in the core interfaces used by the project
  • The codebase primarily uses stable CometBFT APIs (crypto, RPC clients, ABCI types)
  • The DB component upgrade (v0.14.1) maintains compatibility with the core version

The primary usage patterns involve standard interfaces that are stable within the 0.37.x series:

  • ABCI types for consensus interface
  • RPC client implementations
  • Crypto primitives and logging utilities
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any cometbft-specific type assertions or interface implementations
# that might be affected by the upgrade

# Look for cometbft interface implementations
rg -l "interface {" | xargs rg "cometbft"

# Search for cometbft type assertions
ast-grep --pattern 'interface{} = $expr.(github.com/cometbft/$type)'

Length of output: 17007


Script:

#!/bin/bash
# Check for changes in CometBFT interfaces and types between versions
# First, let's check the current version's imports

# Look for interface implementations and type assertions
ast-grep --pattern 'type $name struct {
  $$$
  cometbft.$rest
  $$$
}'

# Check for method implementations that might be affected
ast-grep --pattern 'func ($receiver $type) $method($params) $return {
  $$$
  cometbft.$rest
  $$$
}'

# Search for any specific version constraints or compatibility notes
rg -i "require.*(cometbft.*v0\.37\.(4|12))" -A 5 -B 5

# Look for any migration guides or breaking changes mentioned in docs
fd README.md | xargs rg -i "migration|breaking.*changes|upgrade"

Length of output: 509


Script:

#!/bin/bash
# Let's examine the changelog or release notes for breaking changes
curl -sL https://api.github.com/repos/cometbft/cometbft/releases/tags/v0.37.12 | jq -r '.body'

# Check for any specific version constraints in go.mod
rg -A 10 "\[require\]|require \(" go.mod

# Look for any CometBFT interface implementations
ast-grep --pattern 'interface $name {
  $$$
}'

# Check for any custom implementations of CometBFT types
rg "implements.*cometbft" -B 2 -A 2

Length of output: 1953


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

codecov bot commented Oct 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.47%. Comparing base (d9ee4f3) to head (0e8f2c0).
Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #3045   +/-   ##
========================================
  Coverage    64.47%   64.47%           
========================================
  Files          412      412           
  Lines        28985    28985           
========================================
  Hits         18687    18687           
  Misses        9515     9515           
  Partials       783      783           

@gartnera gartnera force-pushed the upgrade-cometbft-v0.37.12 branch from 21c9127 to b41ff4a Compare October 25, 2024 19:05
@github-actions github-actions bot added the ci Changes to CI pipeline or github actions label Oct 25, 2024
@gartnera gartnera force-pushed the upgrade-cometbft-v0.37.12 branch from b41ff4a to 45b3215 Compare October 28, 2024 22:19
@github-actions github-actions bot removed the ci Changes to CI pipeline or github actions label Oct 28, 2024
@gartnera gartnera removed UPGRADE_TESTS Run make start-upgrade-tests UPGRADE_IMPORT_MAINNET_TESTS Run make start-upgrade-import-mainnet-test labels Oct 28, 2024
@gartnera gartnera marked this pull request as ready for review October 29, 2024 00:02
@gartnera gartnera added this pull request to the merge queue Oct 29, 2024
Merged via the queue into develop with commit 9b285c2 Oct 29, 2024
37 checks passed
@gartnera gartnera deleted the upgrade-cometbft-v0.37.12 branch October 29, 2024 16:26
gartnera added a commit that referenced this pull request Oct 29, 2024
github-merge-queue bot pushed a commit that referenced this pull request Oct 29, 2024
* Revert "chore: upgrade cometbft to v0.37.12 (#3045)"

This reverts commit 9b285c2.

* only revert go.mod changes
melekes added a commit to cometbft/cometbft that referenced this pull request Oct 31, 2024
This cometbft-db upgrade is a breaking change which caused zeta database
corruption (or at least consensus failures) on their cosmos sdk v0.47.x
application when upgrading: zeta-chain/node#3045 (revert with details:
zeta-chain/node#3065)

zeta-chain/node#3045
zeta-chain/node#3065
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog Skip changelog CI check PERFORMANCE_TESTS Run make start-e2e-performance-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants