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

Add BLAKE3 hashing algorithm #12379

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

silvanshade
Copy link

This uses the single-threaded C-based routines from libblake3.

This is not optimal performance-wise but should be a good starting point for nix compatibility with BLAKE3 hashing until a more performant implementation based on the multi-threaded BLAKE3 routines (written in Rust) can be developed.

@Ericson2314

Motivation

See below.

Context

#10600 #11999

@silvanshade silvanshade requested a review from edolstra as a code owner January 29, 2025 20:44
@roberth
Copy link
Member

roberth commented Jan 29, 2025

Nice!
Have you cross-validated this with tvix's use of BLAKE3 in their store implementation?

@silvanshade
Copy link
Author

silvanshade commented Jan 29, 2025

Have you cross-validated this with tvix's use of BLAKE3 in their store implementation?

No, but I'm also not sure if that is a meaningful thing we can do in this context. Maybe you can elaborate if you have something specific in mind?

I don't know much about tvix but from scanning the documentation and source code it appears that their use of BLAKE3 is primarily for their custom data model and protocols around interacting with the content addressable store, which as far as I know is not directly comparable to the equivalent in C++ nix.

It would theoretically be possible to re-implement what they have done for the castore here but it would be a lot more work, beyond the scope of this PR, and certainly beyond my current knowledge of nix internals.

What this PR does is just add BLAKE3 as another hashing algorithm, which could presumably be used for anything nix does internally related to hashing, but at the moment is only really exposed for SRI hashes and via the CLI interface for nix hash.

Surprisingly, tvix doesn't seem to support BLAKE3 for SRI hashes though: https://github.com/tvlfyi/tvix/blob/f8325a64845500916f1e40f5d04466d4c7d1bef6/eval/src/builtins/hash.rs#L19-L29

Copy link
Member

@Ericson2314 Ericson2314 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@Ericson2314
Copy link
Member

I asked in #tvix-dev

This uses the single-threaded C-based routines from libblake3.

This is not optimal performance-wise but should be a good starting point
for nix compatibility with BLAKE3 hashing until a more performant
implementation based on the multi-threaded BLAKE3 routines
(written in Rust) can be developed.
@Kranzes
Copy link
Member

Kranzes commented Jan 30, 2025

Surprisingly, tvix doesn't seem to support BLAKE3 for SRI hashes though: https://github.com/tvlfyi/tvix/blob/f8325a64845500916f1e40f5d04466d4c7d1bef6/eval/src/builtins/hash.rs#L19-L29

Because we're trying to be compatible with with Nix.

@@ -10,6 +10,19 @@ namespace nix {
* hashString
* --------------------------------------------------------------------------*/

Copy link
Contributor

Choose a reason for hiding this comment

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

There doesn't seem to be tests added for the SRI hash formats.

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.

5 participants