cosmos-sdk icon indicating copy to clipboard operation
cosmos-sdk copied to clipboard

feat(crypto): EthSecp256k1 Keys

Open itsdevbear opened this issue 2 years ago • 21 comments

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)

itsdevbear avatar Feb 25 '23 21:02 itsdevbear

TODO: Add SigGasConsume and proto register stuff + general cleanup and more rigorous testing.

itsdevbear avatar Feb 26 '23 17:02 itsdevbear

  • LGPLv3 License from Ethermint and attribution to the Evmos team is missing

which code is copied? looking at the code it seems very generic.

tac0turtle avatar Feb 27 '23 13:02 tac0turtle

talked with injective, we should aim to keep compatibility with existing impls

tac0turtle avatar Feb 28 '23 10:02 tac0turtle

@tac0turtle will add the amino to our implementation I shared with you.

itsdevbear avatar Mar 02 '23 02:03 itsdevbear

@itsdevbear lmk if i can assist in helping with this pr

tac0turtle avatar Mar 06 '23 16:03 tac0turtle

@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

itsdevbear avatar Mar 06 '23 21:03 itsdevbear

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.

itsdevbear avatar Mar 18 '23 17:03 itsdevbear

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

tac0turtle avatar Mar 20 '23 07:03 tac0turtle

Should be good @tac0turtle just wanted to confirm that we are good with keeping duplication here.

If all is well, will polish off today.

itsdevbear avatar Apr 10 '23 12:04 itsdevbear

@tac0turtle first pass, lmk what you think, I'm thinking we can clean things up a bit.

itsdevbear avatar Apr 10 '23 21:04 itsdevbear

TODO: add nocgo for the ethsecp

itsdevbear avatar Apr 10 '23 21:04 itsdevbear

TODO: use https://github.com/ledgerwatch/erigon-lib/blob/main/common/address.go

itsdevbear avatar Apr 19 '23 11:04 itsdevbear

@tac0turtle any chance to get through this? I have time today to wrap it up.

itsdevbear avatar Apr 24 '23 10:04 itsdevbear

@tac0turtle should be blessed now

itsdevbear avatar Apr 28 '23 15:04 itsdevbear

everything good except for the non CGO version, do we need this? @tac0turtle

itsdevbear avatar Apr 28 '23 16:04 itsdevbear

🔥

itsdevbear avatar May 12 '23 12:05 itsdevbear

@calbera add ticket to linear to deprecate ours in Polaris and migrate to this.

itsdevbear avatar May 12 '23 12:05 itsdevbear

Thanks for getting this across @tac0turtle

itsdevbear avatar May 12 '23 12:05 itsdevbear

@calbera can you actually wrap this up, we need to disable the nocgo tests and get the simapp to use cgo.

nocgo is cooked

itsdevbear avatar May 12 '23 14:05 itsdevbear

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.

github-actions[bot] avatar Jun 12 '23 00:06 github-actions[bot]

will get to this hopefully soon :)

itsdevbear avatar Jun 19 '23 18:06 itsdevbear

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.

github-actions[bot] avatar Jul 20 '23 00:07 github-actions[bot]

fuk i still gotta do dis

itsdevbear avatar Jul 20 '23 01:07 itsdevbear

converting to draft, the left over items is to only make it work with cgo and fail on startup without cgo

tac0turtle avatar Aug 17 '23 12:08 tac0turtle

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.

github-actions[bot] avatar Sep 17 '23 00:09 github-actions[bot]