cosmos-sdk
cosmos-sdk copied to clipboard
feat(crypto): EthSecp256k1 Keys
Description
We propose adding Ethereum style secp256k1 signing to the SDK to help improve compatibility with ethereum related projects.
Additional work may be required here to ensure Apache 2.0 is being held, to the best of my knowledge the implementations here are without the use of go-ethereum, some structs used for testing were highly influenced by go-ethereum and potentially should be altered, but I will leave that to the team to decide.
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...
- [ ] included the correct type prefix in the PR title
- [ ] added
!to the type prefix if API or client breaking change - [ ] targeted the correct branch (see PR Targeting)
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for building modules
- [ ] included the necessary unit and integration tests
- [ ] added a changelog entry to
CHANGELOG.md - [ ] included comments for documenting Go code
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed
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...
- [ ] confirmed the correct type prefix in the PR title
- [ ] confirmed
!in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
TODO: Add SigGasConsume and proto register stuff + general cleanup and more rigorous testing.
- LGPLv3 License from Ethermint and attribution to the Evmos team is missing
which code is copied? looking at the code it seems very generic.
talked with injective, we should aim to keep compatibility with existing impls
@tac0turtle will add the amino to our implementation I shared with you.
@itsdevbear lmk if i can assist in helping with this pr
@tac0turtle I will cleanup this week. Just was busy with the launch, once its in a good state some eyes would be great.
TODOS:
- Cleanup comments and add more testing
- Re-add amino
- Add GasConsumeDecorator
update here: We should look into supporting the 65th recovery bit as part of the existing solution that has been in cosmos for years.
Feels silly to support both and there is no real downside to the additional bit. Will help cleanup get signers, signatures etc and make that whole part of the sdk much more simplified.
We should aim to support what chains are using now. In the future we can support it and people can migrate but this sort of breaking chnage is impossible to coordinate with the ecosystem
Should be good @tac0turtle just wanted to confirm that we are good with keeping duplication here.
If all is well, will polish off today.
@tac0turtle first pass, lmk what you think, I'm thinking we can clean things up a bit.
TODO: add nocgo for the ethsecp
TODO: use https://github.com/ledgerwatch/erigon-lib/blob/main/common/address.go
@tac0turtle any chance to get through this? I have time today to wrap it up.
@tac0turtle should be blessed now
everything good except for the non CGO version, do we need this? @tac0turtle
🔥
@calbera add ticket to linear to deprecate ours in Polaris and migrate to this.
Thanks for getting this across @tac0turtle
@calbera can you actually wrap this up, we need to disable the nocgo tests and get the simapp to use cgo.
nocgo is cooked
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
will get to this hopefully soon :)
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
fuk i still gotta do dis
converting to draft, the left over items is to only make it work with cgo and fail on startup without cgo
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.