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 and cosmos-sdk #2934

Merged
merged 5 commits into from
Oct 2, 2024
Merged

Conversation

gartnera
Copy link
Member

@gartnera gartnera commented Sep 27, 2024

Description

Upgrade cosmos-sdk and cometbft to latest minor versions.

  • Close most outstanding security issues (even if they are minor/irrelevent)
  • Unblock Upgrade go-ethereum ethermint#86 because x/exp dependency was removed from cosmos/gogoproto

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:

  • Is a way to get the new rpcimportable test to work without the libsecp256k1_sdk tag. Update: no, there are too many dependencies on on github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1. Graph:
    rpcimportable_secp256k1

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

    • Introduced a new CI job ci-ok for enhanced error handling in the testing workflow.
    • Added a new shell script test.sh for running tests in the rpcimportable module.
  • Bug Fixes

    • Updated the sorting mechanism in the ChainRegistry for improved clarity and consistency.
  • Documentation

    • Adjusted the documentation generation script for zetacored, removing the installation command and streamlining the process.
  • Chores

    • Updated various dependency versions for better stability and performance.

@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 Sep 27, 2024
Copy link
Contributor

coderabbitai bot commented Sep 27, 2024

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough
📝 Walkthrough

Walkthrough

The 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 .golangci.yml for linting exclusions, and adjustments to the Makefile for build flags. A new shell script for testing has been added, and various dependencies in the go.mod file have been updated to their latest versions. Additionally, the sorting mechanism in the ChainRegistry struct has been improved.

Changes

File(s) Change Summary
.github/workflows/ci.yml Added merge_group trigger, introduced ci-ok job, updated test step to use retry mechanism, replaced go test with ./test.sh in rpcimportable job.
.golangci.yml Added libsecp256k1_sdk build tag, set tests option to false, and modified linters and linters-settings for stricter configurations.
Makefile Updated ldflags and BUILD_FLAGS to include new flags, modified TEST_BUILD_FLAGS and HSM_BUILD_FLAGS, and renamed documentation target.
contrib/rpcimportable/test.sh Introduced a new script to run tests with specific flags, ensuring proper execution and error handling.
go.mod Updated various dependencies to newer versions, including github.com/btcsuite/btcd/btcec/v2, github.com/cosmos/cosmos-sdk, and others, reflecting a maintenance effort.
scripts/gen-docs-zetacored.sh Removed installation command, directly invoked zetacored docs for documentation generation, retained file processing logic.
zetaclient/context/chain.go Modified sorting mechanism in the All method of ChainRegistry to use cmp.Compare for clarity and consistency.

Possibly related PRs

Suggested labels

breaking:proto

Suggested reviewers

  • fbac
  • kingpinXD
  • skosito
  • lumtis
  • swift1337
  • brewmaster012

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 Sep 27, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 66.39%. Comparing base (f9cbcfc) to head (84981c2).
Report is 2 commits behind head on develop.

Files with missing lines Patch % Lines
zetaclient/context/chain.go 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #2934   +/-   ##
========================================
  Coverage    66.39%   66.39%           
========================================
  Files          389      389           
  Lines        21758    21758           
========================================
  Hits         14447    14447           
  Misses        6584     6584           
  Partials       727      727           
Files with missing lines Coverage Δ
zetaclient/context/chain.go 76.82% <0.00%> (ø)

@github-actions github-actions bot added the ci Changes to CI pipeline or github actions label Sep 28, 2024
@gartnera gartnera marked this pull request as ready for review September 30, 2024 13:56
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 suggestion

The 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)
    - dot

This addition would further enhance import consistency across the project.

Makefile (2)

Line range hint 17-29: Standardize ldflags Definition and Usage

The 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_sdk

This adjustment ensures all ldflags are included within a single quoted string, preventing potential parsing errors.


31-33: Clarify Comments for Better Understanding

The 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/2243

This 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 dependency gotenv

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

📥 Commits

Files that changed from the base of the PR and between f789138 and 92bc7d1.

⛔ 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 linting

The 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' issues

The 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 of ci-ok job with minor suggestion

The 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 the All 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 tests

Makefile (1)

201-201: Approve Renaming of the Documentation Target

Renaming the target from -docs-zetacored to docs-zetacored aligns with standard naming conventions and improves consistency throughout the Makefile.

This change enhances the readability and maintainability of the build scripts.

go.mod (6)

13-13: Confirm the implications of upgrading btcec/v2 to v2.3.4

Upgrading github.com/btcsuite/btcd/btcec/v2 to v2.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 packages

The OpenTelemetry packages go.opentelemetry.io/otel, go.opentelemetry.io/otel/metric, and go.opentelemetry.io/otel/trace have been updated to v1.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 properly

New 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 updated cometbft version

The github.com/cometbft/cometbft dependency has been updated to v0.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 updating x/crypto

The golang.org/x/crypto package has been updated to v0.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 updated cosmos-sdk and gogoproto dependencies

The github.com/cosmos/cosmos-sdk has been updated to v0.47.14, and github.com/cosmos/gogoproto to v1.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 and gogoproto.


Action Required: Enhance Search for Deprecated APIs in Dependencies

To ensure comprehensive verification of compatibility with the updated cosmos-sdk and gogoproto 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 to v0.47.14 and github.com/cosmos/gogoproto to v1.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

contrib/rpcimportable/test.sh Show resolved Hide resolved
contrib/rpcimportable/test.sh Outdated Show resolved Hide resolved
.github/workflows/ci.yml Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
go.mod Show resolved Hide resolved
go.mod Show resolved Hide resolved
@gartnera gartnera self-assigned this Sep 30, 2024
contrib/rpcimportable/test.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@CryptoFewka CryptoFewka left a 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

@gartnera gartnera added this pull request to the merge queue Oct 2, 2024
Merged via the queue into develop with commit 5d981aa Oct 2, 2024
34 of 35 checks passed
@gartnera gartnera deleted the upgrade-cosmossdk-cometbft branch October 2, 2024 08:44
lumtis added a commit that referenced this pull request Oct 2, 2024
github-merge-queue bot pushed a commit that referenced this pull request Oct 2, 2024
@lumtis lumtis restored the upgrade-cosmossdk-cometbft branch October 2, 2024 14:24
@coderabbitai coderabbitai bot mentioned this pull request Oct 3, 2024
26 tasks
karalabe pushed a commit to ethereum/go-ethereum that referenced this pull request Oct 15, 2024
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
github-merge-queue bot pushed a commit to cometbft/cometbft that referenced this pull request Oct 24, 2024
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>
mergify bot pushed a commit to cometbft/cometbft that referenced this pull request Oct 24, 2024
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
mergify bot pushed a commit to cometbft/cometbft that referenced this pull request Oct 24, 2024
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
mergify bot pushed a commit to cometbft/cometbft that referenced this pull request Oct 24, 2024
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
mergify bot added a commit to cometbft/cometbft that referenced this pull request Oct 24, 2024
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]>
mergify bot added a commit to cometbft/cometbft that referenced this pull request Oct 24, 2024
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]>
mergify bot added a commit to cometbft/cometbft that referenced this pull request Oct 24, 2024
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
mergify bot added a commit to cometbft/cometbft that referenced this pull request Oct 24, 2024
#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]>
mergify bot added a commit to cometbft/cometbft that referenced this pull request Oct 24, 2024
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]>
holiman pushed a commit to ethereum/go-ethereum that referenced this pull request Nov 19, 2024
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Changes to CI pipeline or github actions no-changelog Skip changelog CI check PERFORMANCE_TESTS Run make start-e2e-performance-test UPGRADE_IMPORT_MAINNET_TESTS Run make start-upgrade-import-mainnet-test UPGRADE_TESTS Run make start-upgrade-tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants