-
Notifications
You must be signed in to change notification settings - Fork 26
Digest: Public API (draft) #1140
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
base: main
Are you sure you want to change the base?
Conversation
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.
I'm not sure what the usages are exactly because of the missing docs. We should have public APIs that can do something like
// oneshot, algorithm specific
let digest = Sha256::hash(b"input");
// oneshot, "multiplexed"
let digest = Digest::hash(Algorithm::Sha256, b"input");
// oneshot, "multiplexed" with caller provided memory
let mut digest = [0u8; 32];
Digest::hash(Algorithm::Blake2b, &mut digest, b"input");
// incremental
let mut hasher = Sha256::new();
hasher.update(b"input");
let digest = hasher.finalize();
// or with caller provided memory
hasher.finalize(&mut digest);
// Similar for the "multiplexed" version.Without docs it's hard to tell if this is possible. But looking at the tests, this looks way more complex right now.
| @@ -0,0 +1,3 @@ | |||
| pub trait HashConsts { | |||
| const DIGEST_SIZE: usize; | |||
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.
Shall we add max input length here as well?
| @@ -0,0 +1,104 @@ | |||
| #[derive(Debug)] | |||
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.
Some doc comments would be nice 😊
| } | ||
|
|
||
| impl<Algo> DigestMut<'_, Algo> { | ||
| pub fn algo(&self) -> &Algo { |
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.
Maybe algorithm? algo sounds a little off.
| let algo = Blake2sHash::<RuntimeDigestLen>::default(); | ||
| let digest_mut = DigestMut::new_for_algo(algo, &mut digest).unwrap(); | ||
| algo.hash(digest_mut, b"this is a test").unwrap(); |
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.
This seems like a lot of code for just calling a hash function.
I'd want to call something like Blake2sHash::hash(digest_mut, b"this is a test"). It's not clear why the extra two lines are necessary.
This pull request begins implementing a public Digest API with multiplexing (as part of #1039).
Hasherstruct initialization viaDigestIncrementalBase::new()libcrux_traits::digest::Hasherstructs in newlibcrux-digestcrateHashinlibcrux-digest