-
Notifications
You must be signed in to change notification settings - Fork 463
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
Monorepo ed/x/curve25519 #536
Conversation
Fix alloc feature compilation
Updates the merlin dependency to ^2 and the correct repo
…ature-error Impl std::error::Error for SignatureError.
RFC8032 specifies that the context cannot be greater than 255 octets, but in the previous implementation in ed25519-dalek, this error would only be caught by a debug_assert. This changes the sign_prehashed() function to return a Result so that the error can be handled at runtime and the library no longer allows misuse by creating signatures that other libraries cannot handle.
We're now aliasing SignatureError to the error type from the signature crate.
This is no longer actively breaking our no_std build, but I think it's still technically a minor bug, and further case of issue dalek-cryptography#108
`rand_core` defines a blanket impl of `RngCore + CryptoRng` for `&mut T` where `T: RngCore + CryptoRng`, so rather than requiring a borrowed RNG, it's better to require an owned RNG, as this allows passing either owned or borrowed values. In particular, this makes `OsRng` usage much more ergonomic, because the caller is not forced to do `&mut OsRng` on the zero-sized struct.
Closes dalek-cryptography#59. The doc examples have code interspersed with text explaining the API. Because each doctest executes independently, when these code examples are run as doctests, they have to include parts of the previous examples with # lines. These lines are hidden from Rustdoc output and do not appear in the rendered docs, but they do appear when viewing the README.md on Github. In order to hide these on Github, the code blocks were made non-executable, with their content moved to a unit test. However, this meant that the example API usage was not tested, and so when the unit test was updated to remove the deprecated `rand_os`, there was no check that the examples stayed in sync with the test, causing dalek-cryptography#59. To prevent this from reocurring in the future, go back to executable tests of the API examples.
Add .to_bytes() to PublicKey, SharedSecret
…ve-rand-os-example Remove rand os example
Also does a pass through the docs converting `TypeNames` to Rustdoc links, and making sure that all the items have consistent summaries. Closes dalek-cryptography#58 Closes $56
…ify-ephemeral-static Clarify Ephemeral/StaticSecret docs.
This allows unifying dependencies with other crates using the `3.x` series of the curve library. It is a semver patch-level change, because the x25519-dalek API does not expose any details of the underlying curve implementation.
97625bd
to
6f5281d
Compare
Thank you! Two things:
|
This can be audited with ... rsync :) $ rsync -rcnv a/* b/
e.g. like this: git clone https://github.com/dalek-cryptography/curve25519-dalek.git
git clone https://github.com/dalek-cryptography/ed25519-dalek.git
git clone https://github.com/dalek-cryptography/x5519-dalek.git
pull this pr to "monorepo"
rsync -rcnv monorepo/curve25519-dalek/* curve25519-dalek/
rsync -rcnv monorepo/ed25519-dalek/* ed25519-dalek/
rsync -rcnv monorepo/x25519-dalek/* x25519-dalek/ Above without 'dry run" you can also do "git status" to see if any files changed and how they changed "git diff" if any. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments. Verified that the subdirectories match the current main
branch of all the relevant repos (outside the necessary Cargo.toml changes). Used
diff -r ./curve25519-dalek ../curve25519-dalek
etc.
Alright, made a pretty barebones README, but it should be OK for now imo. The dalek image doesn't load correctly anymore bc the URL is gonna change in this PR. |
Co-authored-by: Michael Rosenberg <[email protected]>
Ok - I had to re-organize our last commits since this PR needs a merge (not a squash) making it more difficult to change here (suggest doing more in follow-up PR's) and we have renames which need to be kept in separate commits in the middle. I gave you co-authored-by since I had to amend our last change commits on top. |
Co-authored-by: Michael Rosenberg <[email protected]>
Co-authored-by: Michael Rosenberg <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotta re-check the subdirs with diff
again bc of the force push. But barring that good to go @tarcieri ?
Update: re-verfified. For concreteness, the current HEADs are:
Build's broken now but I have no idea what's causing it 😕 |
Something broke in transient dependency somewhere for the MSRV build - this should be mergeable in any case as this doesn't cause it. Found what caused it - raised #538 to track it |
As discussed this is the monorepo PR
ed25519
could haveed:
prefix and the PR links would be linked to old place - nonetheless this would be the tradeoff with history we would pay for easier consistency across the crates and usually the original commits has all the information from PR'sgit combine Procedure (see near last commits)