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

Update digest and sha2 versions to 0.10 #2

Merged
merged 1 commit into from
Feb 1, 2022

Conversation

remoun
Copy link

@remoun remoun commented Jan 19, 2022

Also update bincode, criterion and hex, but those are minor.

Supports mobilecoinfoundation/mobilecoin#1326

cbeck88
cbeck88 previously approved these changes Jan 27, 2022
Copy link

@cbeck88 cbeck88 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link

@cbeck88 cbeck88 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM: it would be great to get this in upstream though, maybe we can talk to Isis?

Copy link

@jcape jcape left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, agree with Chris that it's worth trying to get these changes upstream

@remoun remoun merged commit 8791722 into mobilecoinfoundation:mobilecoin Feb 1, 2022
Copy link

@isislovecruft isislovecruft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pull should be made against dalek-cryptography:curve25519-dalek, and then that upstream should be merged here.

Comment on lines -35 to +37
bincode = "1"
criterion = "0.3.0"
hex = "0.4.2"
sha2 = { version = "0.10", default-features = false }
bincode = "1.3.3"
criterion = "0.3.5"
hex = "0.4.3"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are all semver equivalent to my knowledge.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh sorry, not the sha2 change, I don't know why that got included in the diff, just the bincode, criterion, and hex changes.

Copy link
Author

@remoun remoun Feb 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking I'd update the MCF forks in line with what we've been doing, then send another PR merging into upstraem dalek-crypto:master. Would that work?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants