-
Notifications
You must be signed in to change notification settings - Fork 108
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 and cosmos-sdk #2934
Conversation
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces significant changes across multiple files, primarily enhancing the CI workflow, updating dependency versions, and refining testing processes. Key modifications include the addition of a new job in the CI configuration, updates to the Changes
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
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2934 +/- ##
========================================
Coverage 66.39% 66.39%
========================================
Files 389 389
Lines 21758 21758
========================================
Hits 14447 14447
Misses 6584 6584
Partials 727 727
|
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: 6
🧹 Outside diff range and nitpick comments (4)
.golangci.yml (1)
Line range hint
41-49
: Approve GCI linter settings with a suggestionThe updated
gci
linter settings are well-structured and will enforce consistent import ordering. The inclusion of a specific section for the project's imports (prefix(github.com/zeta-chain/node)
) is a good practice.Consider adding a
dot-import
section to explicitly handle and potentially discourage dot imports, which can sometimes lead to unclear code:gci: sections: - standard - default - prefix(github.com/zeta-chain/node) - dotThis addition would further enhance import consistency across the project.
Makefile (2)
Line range hint
17-29
: Standardizeldflags
Definition and UsageThe
ldflags
variable currently mixes quoted and unquoted flags, which can lead to parsing issues or unintended behavior during the build process. Specifically,-extldflags=-Wl,--allow-multiple-definition
is appended outside the quoted flags.For clarity and to ensure all linker flags are correctly passed, consider encapsulating all flags within a single set of quotes. This approach maintains consistency and enhances readability across the build configurations.
Apply the following refactor:
ldflags = \ - -X github.com/cosmos/cosmos-sdk/version.Name=zetacore \ - -X github.com/cosmos/cosmos-sdk/version.ServerName=zetacored \ - -X github.com/cosmos/cosmos-sdk/version.ClientName=zetaclientd \ - -X github.com/cosmos/cosmos-sdk/version.Version=$(VERSION) \ - -X github.com/cosmos/cosmos-sdk/version.Commit=$(COMMIT) \ - -X github.com/zeta-chain/node/pkg/constant.Name=zetacored \ - -X github.com/zeta-chain/node/pkg/constant.Version=$(VERSION) \ - -X github.com/zeta-chain/node/pkg/constant.CommitHash=$(COMMIT) \ - -X github.com/zeta-chain/node/pkg/constant.BuildTime=$(BUILDTIME) \ - -X github.com/cosmos/cosmos-sdk/types.DBBackend=pebbledb \ - -extldflags=-Wl,--allow-multiple-definition + '-X github.com/cosmos/cosmos-sdk/version.Name=zetacore \ + -X github.com/cosmos/cosmos-sdk/version.ServerName=zetacored \ + -X github.com/cosmos/cosmos-sdk/version.ClientName=zetaclientd \ + -X github.com/cosmos/cosmos-sdk/version.Version=$(VERSION) \ + -X github.com/cosmos/cosmos-sdk/version.Commit=$(COMMIT) \ + -X github.com/zeta-chain/node/pkg/constant.Name=zetacored \ + -X github.com/zeta-chain/node/pkg/constant.Version=$(VERSION) \ + -X github.com/zeta-chain/node/pkg/constant.CommitHash=$(COMMIT) \ + -X github.com/zeta-chain/node/pkg/constant.BuildTime=$(BUILDTIME) \ + -X github.com/cosmos/cosmos-sdk/types.DBBackend=pebbledb \ + -extldflags=-Wl,--allow-multiple-definition' BUILD_FLAGS := -ldflags $(ldflags) -tags pebbledb,ledger,libsecp256k1_sdkThis adjustment ensures all
ldflags
are included within a single quoted string, preventing potential parsing errors.
31-33
: Clarify Comments for Better UnderstandingThe comments explaining the inclusion of
libsecp256k1_sdk
can be enhanced for clarity and professionalism.Consider rephrasing the comments for better readability:
-# enable libsecp256k1_sdk to bypass the btcec breaking change: +# Enable `libsecp256k1_sdk` to bypass the `btcec` breaking change. -# https://github.com/btcsuite/btcd/issues/2243 +# Reference: https://github.com/btcsuite/btcd/issues/2243This provides a clearer explanation of the purpose and directs readers to the relevant issue for more information.
go.mod (1)
281-281
: Consider removing unused indirect dependencygotenv
The indirect dependency
github.com/subosito/gotenv v1.6.0
is listed. If this package is no longer utilized within the project, it would be prudent to remove it to streamline the dependency graph and minimize potential security risks from unused packages.
📜 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 (7)
- .github/workflows/ci.yml (1 hunks)
- .golangci.yml (1 hunks)
- Makefile (3 hunks)
- contrib/rpcimportable/test.sh (1 hunks)
- go.mod (13 hunks)
- scripts/gen-docs-zetacored.sh (0 hunks)
- zetaclient/context/chain.go (2 hunks)
💤 Files with no reviewable changes (1)
- scripts/gen-docs-zetacored.sh
🧰 Additional context used
📓 Path-based instructions (2)
contrib/rpcimportable/test.sh (1)
Pattern
**/*.sh
: Review the shell scripts, point out issues relative to security, performance, and maintainability.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-69: zetaclient/context/chain.go#L69
Added line #L69 was not covered by tests
🔇 Additional comments (13)
contrib/rpcimportable/test.sh (1)
1-5
: Appropriate shebang and shell options.The script correctly uses
#!/bin/bash
and sets appropriate shell options (-e
and-o pipefail
) for robust error handling. This ensures that the script fails fast and visibly on any error, which is crucial for maintaining reliability in automated testing environments..golangci.yml (2)
3-6
: Consider the implications of excluding tests from lintingThe addition of
tests: false
excludes test files from linting. While this can speed up the linting process, it may lead to inconsistencies or issues in test code going undetected. Consider the trade-offs carefully.The inclusion of the
libsecp256k1_sdk
build tag aligns with the PR objectives and is appropriate for the transition to CGO secp256k1.To ensure that excluding tests from linting doesn't introduce inconsistencies, run the following script:
Line range hint
51-54
: Verify the necessity of excluding 'composite' issuesThe exclusion of 'composite' issues from linting warrants further investigation. This exclusion might potentially hide important issues in the codebase.
Could you please provide more context on why this exclusion is necessary? If it's addressing a specific known issue, consider adding a comment in the configuration file explaining the rationale.
If this exclusion is no longer needed, it would be beneficial to remove it to ensure more comprehensive linting coverage.
To assess the impact of this exclusion, run the following script:
This will help determine if there are any composite issues that should be addressed rather than excluded.
.github/workflows/ci.yml (1)
Line range hint
119-129
: Approval ofci-ok
job with minor suggestionThe addition of the
ci-ok
job is a commendable improvement to the CI workflow. It provides a clear, final status check that depends on the success of all previous jobs. This approach simplifies the process of determining the overall success of the CI run.To further enhance clarity and maintainability, consider the following minor suggestion:
ci-ok: runs-on: ubuntu-22.04 needs: - build-and-test - rpcimportable if: always() steps: - if: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') }} run: | - echo "One of the jobs failed or was cancelled" + echo "CI failed: One or more jobs failed or were cancelled" exit 1 + - if: ${{ !contains(needs.*.result, 'failure') && !contains(needs.*.result, 'cancelled') }} + run: echo "CI passed successfully"This modification provides explicit success and failure messages, improving the readability of CI logs.
zetaclient/context/chain.go (2)
4-4
: Import addition is appropriate.The addition of the
cmp
package import is necessary and aligns with the changes made to the sorting function in theAll
method. This import enhances the code's functionality by providing more robust comparison capabilities.
69-69
: Improved sorting mechanism, but lacks test coverage.The modification to use
cmp.Compare
in the sorting function is a commendable improvement. It enhances the sorting precision and aligns with Go's standard library conventions, promoting better readability and maintainability.However, it's crucial to note that this line is not covered by tests according to the static analysis report.
To ensure the robustness of this change, please add test coverage for this line. Consider adding a test case that verifies the correct sorting order of chains in various scenarios, including edge cases with similar chain IDs.
You can use the following script to check the current test coverage for this file:
This will help identify the specific coverage for the
chain.go
file and guide your efforts in improving test coverage.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 69-69: zetaclient/context/chain.go#L69
Added line #L69 was not covered by testsMakefile (1)
201-201
: Approve Renaming of the Documentation TargetRenaming the target from
-docs-zetacored
todocs-zetacored
aligns with standard naming conventions and improves consistency throughout theMakefile
.This change enhances the readability and maintainability of the build scripts.
go.mod (6)
13-13
: Confirm the implications of upgradingbtcec/v2
tov2.3.4
Upgrading
github.com/btcsuite/btcd/btcec/v2
tov2.3.4
ensures that the project benefits from the latest security patches and performance improvements. It's advisable to confirm that cryptographic operations remain consistent and that there are no behavioral changes affecting key functionalities.
306-308
: Ensure consistent OpenTelemetry instrumentation with updated packagesThe OpenTelemetry packages
go.opentelemetry.io/otel
,go.opentelemetry.io/otel/metric
, andgo.opentelemetry.io/otel/trace
have been updated tov1.24.0
. Verify that all OpenTelemetry components and instrumentation within the project are compatible with this version, and update any initialization code if necessary.
349-350
: Integrate new OpenTelemetry instrumentation packages properlyNew dependencies have been added:
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.49.0
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.49.0
Ensure that these instrumentation packages are correctly initialized and integrated within your application to leverage enhanced tracing and monitoring capabilities.
19-19
: Assess integration with the updatedcometbft
versionThe
github.com/cometbft/cometbft
dependency has been updated tov0.37.11
. This update may include critical bug fixes or changes in consensus mechanisms. It's important to verify that these changes do not introduce any unexpected behavior in the application's consensus processes.Please execute the following script to check for API changes:
#!/bin/bash # Description: Identify any breaking changes in cometbft APIs. # Test: Search for usages of cometbft functions that may have changed. # Expected: Review matches for compatibility issues. rg --type go 'cometbft\.(OldFunction|ChangedFunction)' .
65-65
: Validate cryptographic functionalities after updatingx/crypto
The
golang.org/x/crypto
package has been updated tov0.26.0
. This package is critical for encryption, hashing, and other security-related functions. Ensure that all cryptographic operations perform as expected with the new version, and that there are no deprecated methods in use.Please run the following script to identify any usage of deprecated crypto functions:
#!/bin/bash # Description: Find usages of potentially deprecated crypto functions. # Test: Search for deprecated or altered functions in x/crypto. # Expected: No matches indicating deprecated usage. rg --type go 'crypto.(MD4|MD5|SHA1)' .
22-23
: Verify compatibility with updatedcosmos-sdk
andgogoproto
dependenciesThe
github.com/cosmos/cosmos-sdk
has been updated tov0.47.14
, andgithub.com/cosmos/gogoproto
tov1.7.0
. While these are minor version upgrades, it is crucial to ensure that the codebase remains compatible with these versions. Potential deprecations or changes in APIs may affect functionality.Please run the following script to identify any deprecated or modified APIs:
✅ Verification successful
Action Required: Correct and Re-run API Deprecation Checks
The previous script encountered a syntax error due to an improperly formatted regex pattern. Please execute the corrected script below to identify any deprecated or modified APIs in
cosmos-sdk
andgogoproto
.
Action Required: Enhance Search for Deprecated APIs in Dependencies
To ensure comprehensive verification of compatibility with the updated
cosmos-sdk
andgogoproto
dependencies, please execute the enhanced search script below. This script broadens the search criteria to identify any deprecated functions or related annotations within the codebase.
Compatibility Verified: No Deprecated APIs from Updated Dependencies Found
The analysis confirms that updating
github.com/cosmos/cosmos-sdk
tov0.47.14
andgithub.com/cosmos/gogoproto
tov1.7.0
does not introduce any deprecated or modified API usages within the codebase. Existing deprecation annotations are confined to the project's internal modules and do not impact the updated dependencies.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for usages of deprecated or changed APIs in cosmos-sdk and gogoproto. # Test: Find deprecated functions or packages. # Expected: No matches indicating deprecated API usage. rg --type go 'cosmos-sdk.*DeprecatedFunction|gogoproto.*OldFunction' .Length of output: 195
Script:
#!/bin/bash # Description: Search for usages of deprecated or changed APIs in cosmos-sdk and gogoproto. # Corrected regex pattern to avoid syntax errors. rg --type go 'cosmos-sdk\..*DeprecatedFunction|gogoproto\..*OldFunction' .Length of output: 76
Script:
#!/bin/bash # Description: Enhanced search for deprecated or changed APIs in cosmos-sdk and gogoproto. # Search for any usage of deprecated functions or annotations in comments. rg --type go '(cosmos-sdk\..*Deprecated|gogoproto\..*Deprecated|//\s*Deprecated)' . # Additionally, use ast-grep to find deprecated function calls. ast-grep --lang go --pattern ' (call_expr function: (selector_expr field: "DeprecatedFunction" | "OldFunction" ) ) ' .Length of output: 7936
92bc7d1
to
84981c2
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.
CI changes look good to me
Use `github.com/decred/dcrd/dcrec/secp256k1/v4` directly rather than `github.com/btcsuite/btcd/btcec/v2` which is just a wrapper around the underlying decred library. Inspired by cosmos/cosmos-sdk#15018 `github.com/btcsuite/btcd/btcec/v2` has a very annoying breaking change when upgrading from `v2.3.3` to `v2.3.4`. The easiest way to workaround this is to just remove the wrapper. Would be very nice if you could backport this to the release branches. References: - btcsuite/btcd#2221 - cometbft/cometbft#4294 - cometbft/cometbft#3728 - zeta-chain/node#2934
Use `github.com/decred/dcrd/dcrec/secp256k1/v4` directly rather than `github.com/btcsuite/btcd/btcec/v2` which is just a wrapper around the underlying decred library. Inspired by cosmos/cosmos-sdk#15018 `github.com/btcsuite/btcd/btcec/v2` has a very annoying breaking change when upgrading from `v2.3.3` to `v2.3.4`. The easiest way to workaround this is to just remove the wrapper. Would be very nice if you could backport this to v0.37.x and v0.38.x. References: - btcsuite/btcd#2221 - #3728 - zeta-chain/node#2934 --- #### PR checklist - [ ] Tests written/updated - [x] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments --------- Co-authored-by: Andy Nogueira <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Use `github.com/decred/dcrd/dcrec/secp256k1/v4` directly rather than `github.com/btcsuite/btcd/btcec/v2` which is just a wrapper around the underlying decred library. Inspired by cosmos/cosmos-sdk#15018 `github.com/btcsuite/btcd/btcec/v2` has a very annoying breaking change when upgrading from `v2.3.3` to `v2.3.4`. The easiest way to workaround this is to just remove the wrapper. Would be very nice if you could backport this to v0.37.x and v0.38.x. References: - btcsuite/btcd#2221 - #3728 - zeta-chain/node#2934 --- #### PR checklist - [ ] Tests written/updated - [x] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments --------- Co-authored-by: Andy Nogueira <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit 9b8eafa) # Conflicts: # crypto/secp256k1/secp256k1.go # go.mod
Use `github.com/decred/dcrd/dcrec/secp256k1/v4` directly rather than `github.com/btcsuite/btcd/btcec/v2` which is just a wrapper around the underlying decred library. Inspired by cosmos/cosmos-sdk#15018 `github.com/btcsuite/btcd/btcec/v2` has a very annoying breaking change when upgrading from `v2.3.3` to `v2.3.4`. The easiest way to workaround this is to just remove the wrapper. Would be very nice if you could backport this to v0.37.x and v0.38.x. References: - btcsuite/btcd#2221 - #3728 - zeta-chain/node#2934 --- #### PR checklist - [ ] Tests written/updated - [x] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments --------- Co-authored-by: Andy Nogueira <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit 9b8eafa) # Conflicts: # .golangci.yml # crypto/secp256k1/secp256k1.go # crypto/secp256k1/secp256k1_internal_test.go # crypto/secp256k1/secp256k1_test.go # go.mod
Use `github.com/decred/dcrd/dcrec/secp256k1/v4` directly rather than `github.com/btcsuite/btcd/btcec/v2` which is just a wrapper around the underlying decred library. Inspired by cosmos/cosmos-sdk#15018 `github.com/btcsuite/btcd/btcec/v2` has a very annoying breaking change when upgrading from `v2.3.3` to `v2.3.4`. The easiest way to workaround this is to just remove the wrapper. Would be very nice if you could backport this to v0.37.x and v0.38.x. References: - btcsuite/btcd#2221 - #3728 - zeta-chain/node#2934 --- #### PR checklist - [ ] Tests written/updated - [x] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments --------- Co-authored-by: Andy Nogueira <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit 9b8eafa) # Conflicts: # .changelog/v0.37.3/features/4294-remove-secp256k1-wrapper.md # .golangci.yml # crypto/secp256k1/secp256k1.go # crypto/secp256k1/secp256k1_internal_test.go # crypto/secp256k1/secp256k1_test.go # go.mod # go.sum
Use `github.com/decred/dcrd/dcrec/secp256k1/v4` directly rather than `github.com/btcsuite/btcd/btcec/v2` which is just a wrapper around the underlying decred library. Inspired by cosmos/cosmos-sdk#15018 `github.com/btcsuite/btcd/btcec/v2` has a very annoying breaking change when upgrading from `v2.3.3` to `v2.3.4`. The easiest way to workaround this is to just remove the wrapper. Would be very nice if you could backport this to v0.37.x and v0.38.x. References: - btcsuite/btcd#2221 - #3728 - zeta-chain/node#2934 --- #### PR checklist - [ ] Tests written/updated - [x] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments <hr>This is an automatic backport of pull request #4294 done by [Mergify](https://mergify.com). --------- Co-authored-by: Alex Gartner <[email protected]> Co-authored-by: Anton Kaliaev <[email protected]>
Use `github.com/decred/dcrd/dcrec/secp256k1/v4` directly rather than `github.com/btcsuite/btcd/btcec/v2` which is just a wrapper around the underlying decred library. Inspired by cosmos/cosmos-sdk#15018 `github.com/btcsuite/btcd/btcec/v2` has a very annoying breaking change when upgrading from `v2.3.3` to `v2.3.4`. The easiest way to workaround this is to just remove the wrapper. Would be very nice if you could backport this to v0.37.x and v0.38.x. References: - btcsuite/btcd#2221 - #3728 - zeta-chain/node#2934 --- #### PR checklist - [ ] Tests written/updated - [x] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments <hr>This is an automatic backport of pull request #4294 done by [Mergify](https://mergify.com). --------- Co-authored-by: Alex Gartner <[email protected]> Co-authored-by: Anton Kaliaev <[email protected]>
Use `github.com/decred/dcrd/dcrec/secp256k1/v4` directly rather than `github.com/btcsuite/btcd/btcec/v2` which is just a wrapper around the underlying decred library. Inspired by cosmos/cosmos-sdk#15018 `github.com/btcsuite/btcd/btcec/v2` has a very annoying breaking change when upgrading from `v2.3.3` to `v2.3.4`. The easiest way to workaround this is to just remove the wrapper. Would be very nice if you could backport this to v0.37.x and v0.38.x. References: - btcsuite/btcd#2221 - #3728 - zeta-chain/node#2934 --- #### PR checklist - [ ] Tests written/updated - [x] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments <hr>This is an automatic backport of pull request #4294 done by [Mergify](https://mergify.com). --------- Co-authored-by: Alex Gartner <[email protected]> Co-authored-by: Anton Kaliaev <[email protected]> (cherry picked from commit 048bb7a) # Conflicts: # go.mod # go.sum
#4330) Use `github.com/decred/dcrd/dcrec/secp256k1/v4` directly rather than `github.com/btcsuite/btcd/btcec/v2` which is just a wrapper around the underlying decred library. Inspired by cosmos/cosmos-sdk#15018 `github.com/btcsuite/btcd/btcec/v2` has a very annoying breaking change when upgrading from `v2.3.3` to `v2.3.4`. The easiest way to workaround this is to just remove the wrapper. Would be very nice if you could backport this to v0.37.x and v0.38.x. References: - btcsuite/btcd#2221 - #3728 - zeta-chain/node#2934 --- #### PR checklist - [ ] Tests written/updated - [x] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments <hr>This is an automatic backport of pull request #4294 done by [Mergify](https://mergify.com).<hr>This is an automatic backport of pull request #4328 done by [Mergify](https://mergify.com). --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Anton Kaliaev <[email protected]>
Use `github.com/decred/dcrd/dcrec/secp256k1/v4` directly rather than `github.com/btcsuite/btcd/btcec/v2` which is just a wrapper around the underlying decred library. Inspired by cosmos/cosmos-sdk#15018 `github.com/btcsuite/btcd/btcec/v2` has a very annoying breaking change when upgrading from `v2.3.3` to `v2.3.4`. The easiest way to workaround this is to just remove the wrapper. Would be very nice if you could backport this to v0.37.x and v0.38.x. References: - btcsuite/btcd#2221 - #3728 - zeta-chain/node#2934 --- #### PR checklist - [ ] Tests written/updated - [x] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments <hr>This is an automatic backport of pull request #4294 done by [Mergify](https://mergify.com). --------- Co-authored-by: Alex Gartner <[email protected]> Co-authored-by: Anton Kaliaev <[email protected]>
Use `github.com/decred/dcrd/dcrec/secp256k1/v4` directly rather than `github.com/btcsuite/btcd/btcec/v2` which is just a wrapper around the underlying decred library. Inspired by cosmos/cosmos-sdk#15018 `github.com/btcsuite/btcd/btcec/v2` has a very annoying breaking change when upgrading from `v2.3.3` to `v2.3.4`. The easiest way to workaround this is to just remove the wrapper. Would be very nice if you could backport this to the release branches. References: - btcsuite/btcd#2221 - cometbft/cometbft#4294 - cometbft/cometbft#3728 - zeta-chain/node#2934
Use `github.com/decred/dcrd/dcrec/secp256k1/v4` directly rather than `github.com/btcsuite/btcd/btcec/v2` which is just a wrapper around the underlying decred library. Inspired by cosmos/cosmos-sdk#15018 `github.com/btcsuite/btcd/btcec/v2` has a very annoying breaking change when upgrading from `v2.3.3` to `v2.3.4`. The easiest way to workaround this is to just remove the wrapper. Would be very nice if you could backport this to the release branches. References: - btcsuite/btcd#2221 - cometbft/cometbft#4294 - cometbft/cometbft#3728 - zeta-chain/node#2934
Description
Upgrade cosmos-sdk and cometbft to latest minor versions.
We have to switch to using CGO secp256k1 to bypass btcsuite/btcd#2243 and we cannot use native btcec secp256k1 until we upgrade to cosmos-sdk v0.50.x. This doesn't really matter as our release binaries have always been built with CGO enabled.
libsecp256k1_sdk
enables cosmos-sdk to use the same secp256k1 logic that go-ethereum uses.This should also get us in a much better position for the cosmos-sdk v0.50.x upgrade.
We also have the option of only upgrading cosmos-sdk and ignoring the cometbft upgrade for now which should unblock zeta-chain/ethermint#86 (but not resolve any of the cometbft security issues).
TODO:
rpcimportable
test to work without thelibsecp256k1_sdk
tag. Update: no, there are too many dependencies on ongithub.com/cosmos/cosmos-sdk/crypto/keys/secp256k1
. Graph:How Has This Been Tested?
Summary by CodeRabbit
New Features
ci-ok
for enhanced error handling in the testing workflow.test.sh
for running tests in therpcimportable
module.Bug Fixes
ChainRegistry
for improved clarity and consistency.Documentation
zetacored
, removing the installation command and streamlining the process.Chores