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

Change ed25519 and bcrypt to be forked off golang/x/crypto #1959

Closed
ValarDragon opened this issue Jul 11, 2018 · 5 comments
Closed

Change ed25519 and bcrypt to be forked off golang/x/crypto #1959

ValarDragon opened this issue Jul 11, 2018 · 5 comments
Labels
C:crypto Component: Crypto S:proposal Status: Proposal T:security Type: Security (specify priority)
Milestone

Comments

@ValarDragon
Copy link
Contributor

Ref: tendermint/go-crypto#60

/cc @ebuchman

@ValarDragon ValarDragon added S:proposal Status: Proposal C:crypto Component: Crypto labels Jul 11, 2018
@ValarDragon
Copy link
Contributor Author

I can see two ways of doing this. One is we fork the entire x/crypto, redo our changes, and see the upstream updates directly.

The other is we keep our current fork of ed25519 (agl's library, x/crypto is basically the same + 2 updates which I've already made PR's for). We also then fork bcrypt and maintain that as a repo.

I think having the seperate ed25519 and bcrypt forks is fine. The issue we ran into with the tendermint repos is that we were developing actively on all of them at once. With ed25519/bcrypt, they should be extremely stable. (Any updates they have should not be breaking)

@xla xla added the T:security Type: Security (specify priority) label Aug 2, 2018
@xla xla added this to the launch milestone Aug 2, 2018
@xla xla changed the title crypto: Change ed25519 and bcrypt to be forked off golang/x/crypto Change ed25519 and bcrypt to be forked off golang/x/crypto Aug 2, 2018
@ebuchman
Copy link
Contributor

Why would we keep separate forks?

@ValarDragon
Copy link
Contributor Author

We could have a single x/crypto fork, I'm not at all opposed to that.

@ebuchman
Copy link
Contributor

ebuchman commented Aug 20, 2018

Ok I made one: https://github.com/tendermint/crypto

It has the update for bcrypt and I updated cosmos/cosmos-sdk#2090 to use it.

Also opened a fork of bip39 rather than copying it in. So cosmos/cosmos-sdk#2090 has both pkgs removed :)

@ebuchman
Copy link
Contributor

ebuchman commented Oct 6, 2018

Closing for #2558

@ebuchman ebuchman closed this as completed Oct 6, 2018
firelizzard18 pushed a commit to AccumulateNetwork/tendermint that referenced this issue Feb 1, 2024
* Disable undesired linting code

* Added unclog entry
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:crypto Component: Crypto S:proposal Status: Proposal T:security Type: Security (specify priority)
Projects
None yet
Development

No branches or pull requests

3 participants