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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add blake3 #8

Merged
merged 5 commits into from
Jan 25, 2024
Merged

Add blake3 #8

merged 5 commits into from
Jan 25, 2024

Conversation

lukechampine
Copy link
Contributor

@lukechampine lukechampine commented Sep 28, 2023

Adds functions corresponding to the exported blake3 arms in urbit/urbit#6802

I'm out of my depth when it comes to C build systems, so I will need help getting this to build... 馃槄

@matthew-levan
Copy link
Collaborator

Nice, this should be done soon. I just pushed a commit that hopefully gets you a little further along the build process. Still hitting 13 errors (mostly related to undefined symbols) during make, but do check out how I updated configure.ac and Makefile.am to continue this work.

@matthew-levan
Copy link
Collaborator

@lukechampine Should work now.

@lukechampine
Copy link
Contributor Author

Thanks! Some thoughts (mostly reminders for myself):

  • output_t is kinda big; we should either ensure that calls to urcrypt_blake3_chunk_output and urcrypt_blake3_parent_output are inlined, or pass an outparam instead
  • urcrypt__reverse isn't called on anything within output_t; this is probably fine since callers should be treating output_t as opaque anyway, but something to be aware of
  • For maximum performance, we will want to call blake3_hash_many, which hashes up to 8 chunks in parallel using SIMD. Perhaps we should expose that instead of urcrypt_blake3_chunk_output, which only hashes 1 chunk (and requires us to export some blake3 guts that weren't meant to be exported)

I'll try to get some jets working using your msl/fetch-urcrypt branch soon :^)

@lukechampine
Copy link
Contributor Author

Planning to revisit this soon in light of the verified streaming work that I recently finished.

I think we really only need three jets:

  • +hash:blake3, for general purpose usage (and maximum throughput for large messages)
  • +chunk-output:blake3, for turning a chunk into an output
  • +compress:blake3, for turning an output into bytes

Importantly, the +chunk-output jet should not return an output_t -- that ends up being more trouble than it's worth. Instead it can return just the cv; the client can figure out the rest of the fields itself.

@lukechampine lukechampine marked this pull request as ready for review November 23, 2023 02:55
@lukechampine lukechampine requested a review from a team as a code owner November 23, 2023 02:55
@lukechampine
Copy link
Contributor Author

@matthew-levan this should be good to merge!

Copy link
Member

@joemfb joemfb left a comment

Choose a reason for hiding this comment

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

this will do

Copy link
Member

@joemfb joemfb left a comment

Choose a reason for hiding this comment

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

this will do

@joemfb joemfb merged commit 9ae5d60 into urbit:master Jan 25, 2024
joemfb added a commit to urbit/vere that referenced this pull request Jan 25, 2024
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.

None yet

3 participants