-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
fix(crypto): secp256k1 cgo duplicate symbols #18306
Conversation
Note Currently processing new changes in this PR. This may take a few minutes, please wait... Files selected for processing (26)
TipsChat with CodeRabbit Bot (
|
@JulianToledano your pull request is missing a changelog! |
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.
is there any way to do this without changing the fork?
potentially creating another file that can be used when importing geth, different build tag. Preferably it would be good to not touch the fork as when we go to update it this will get reverted
I didn't find any other solution but renaming the duplicated symbols. Problem is |
maybe that is fine, for linux we say use these flags if importing geth and for mac we say we dont support secp cgo while importing geth? this allows us to keep the fork untouched. What are your thoughts on that? |
I've been checking the history of the fork and had never been updated. There are also some commits (most of them about linting and typos) but also adding the I think the decision here is either:
|
im more in favor of not touching forks like this, i think documentation would help. If users would want to modify use it on mac they could do so within docker |
Understood, I'll make a PR with the documentation |
Description
Closes:
#18232
This PR rename all duplicate symbols that any project using the SDK alongside with go-ethereum could get
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
to the type prefix if API or client breaking changeCHANGELOG.md
make lint
andmake test
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!
in the type prefix if API or client breaking changeSummary by CodeRabbit
Refactor:
secp256k1
has been replaced withcosmos_secp256k1
across the codebase. This change is internal and should not affect the end-user experience.Style:
secp256k1
library have been updated to include the prefix "cosmos_". This change is part of the codebase's internal refactoring and does not impact the functionality from an end-user perspective.Documentation:
secp256k1
tocosmos_secp256k1
. This change is primarily for developers and does not affect end-users.