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

crypto/keys: move tendermint key types to the SDK #5868

Closed
wants to merge 26 commits into from

Conversation

fedekunze
Copy link
Collaborator

@fedekunze fedekunze commented Mar 24, 2020

Closes: #5819

Description

Defines Ed25519, Secp256k1, Sr25519 and Multisignature pubkey and privkeys as proto messages.

Dependency migration from Tendermint will come on another PR.

TODO:

  • .proto file for types and interfaces
  • function migration
  • util function to return byte array instead of slice
  • test key compatibility

For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@fedekunze fedekunze added the WIP label Mar 24, 2020
@fedekunze fedekunze added C:Keys Keybase, KMS and HSMs C:Encoding labels Mar 25, 2020
@lgtm-com
Copy link

lgtm-com bot commented Mar 25, 2020

This pull request introduces 2 alerts when merging 5e4724d into c36c9f1 - view on LGTM.com

new alerts:

  • 1 for Unreachable statement
  • 1 for Expression has no effect

@fedekunze fedekunze changed the base branch from fedekunze/5819-move-keys to master March 25, 2020 16:01
@tac0turtle
Copy link
Member

This PR may need to be broken down by keys. This PR is growing to be quite large and these are important things to get right and should be reviewed in depth.

@tac0turtle
Copy link
Member

I believe all that is needed is to define a proto type and import the tendermint types as the used types when casting. will make the needed changes for this

@tac0turtle
Copy link
Member

this is a simpler approach #5997 let me know if this works

@fedekunze
Copy link
Collaborator Author

Replaced by #5997

@fedekunze fedekunze closed this Apr 15, 2020
@fedekunze fedekunze deleted the fedekunze/5819-tm-keys branch April 15, 2020 17:33
@tac0turtle tac0turtle mentioned this pull request Apr 15, 2020
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:Encoding C:Keys Keybase, KMS and HSMs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

proposal: move keys to cosmos-sdk
2 participants