Skip to content

digest: promote blake3 to first-class digest #66

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

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

Conversation

stevvooe
Copy link
Contributor

The dual module approach for blake3 was slightly awkward. Since it
provides similar usability with a massive bump in performance, it's
extremely likely to land as a registered algorithm in the image-spec.

This PR removes the secondary module, which made it difficult to test as
a unit. This may break users who are using HEAD versions of the package.
For a new release, this will be backwards compatible. The other drawback
is that the zeebo/blake3 will now be a dependency but this can be
replaced transparently by the standard libary in the future.

Signed-off-by: Stephen Day stephen.day@getcruise.com

@stevvooe stevvooe force-pushed the blake3-first-class branch from 66e27ab to 1728461 Compare August 19, 2021 01:52
@stevvooe stevvooe requested a review from vbatts August 19, 2021 01:52
Copy link
Member

@vbatts vbatts left a comment

Choose a reason for hiding this comment

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

🍨

@vbatts
Copy link
Member

vbatts commented Sep 23, 2021

oh beans, a rebase is needed

@stevvooe stevvooe requested a review from vbatts November 11, 2021 18:20
The dual module approach for blake3 was slightly awkward. Since it
provides similar usability with a massive bump in performance, it's
extremely likely to land as a registered algorithm in the image-spec.

This PR removes the secondary module, which made it difficult to test as
a unit. This may break users who are using HEAD versions of the package.
For a new release, this will be backwards compatible. The other drawback
is that the zeebo/blake3 will now be a dependency but this can be
replaced transparently by the standard libary in the future.

In addition to promoting blake3, this makes a few style adjustments to
be in line with Go's style guidelines.

Signed-off-by: Stephen Day <stephen.day@getcruise.com>
@AkihiroSuda
Copy link
Member

it's extremely likely to land as a registered algorithm in the image-spec.

I'm not against this, but I'm not sure we have reached the consensus on the algorithm name.
This PR uses "blake3" as the algo name, but opencontainers/image-spec#819 proposes "b3-256".

Is there any actual implementation that has been already using blake3/b3-256/whatever else?

@ktarplee
Copy link

ktarplee commented Dec 4, 2021

Since blake3 supports multiple digests sizes it seems to me that "blake3" without the digest size (e.g., "blake3-256") is a bad choice for a name. Furthermore, it makes sense to register the 512 bit algorithm as well to have the same collision resistance as SHA512 that you already have registered. "blake3-512" or "b3-512" along with either "blake3-256" or "b3-256" seem to make the most sense for names. This is not a binary compact format like multihash so I favor spelling out "blake3" instead of using "b3". That is my two cents for what it is worth.

@AkihiroSuda
Copy link
Member

@stevvooe @dmcgowan What should we do with the algo name?

@vbatts
Copy link
Member

vbatts commented Feb 16, 2022

since the library you've pulled is is defaulting to 256bit, I think going with @ktarplee is a fine idea.
Make the algo name "blake3-256".

And optionally we could also add "blake3-512"

@stevvooe
Copy link
Contributor Author

Do we want to pick this up again? Do we have consensus in the spec yet?

@AkihiroSuda
Copy link
Member

Is there any existing user of blake3 in OCI?
What algo identifier do they use?

@bryantbiggs
Copy link

it would be great to see blake3 usage to start permeating the ecosystem - @ktarplee's naming proposal seems logical

@tianon
Copy link
Member

tianon commented Feb 6, 2025

I think we should codify it in the spec before promotion (which I'm now explicitly in favor of: opencontainers/image-spec#819 (comment)), but with the caveat that I'm only an image-spec maintainer, not go-digest.

@tianon
Copy link
Member

tianon commented Apr 24, 2025

FWIW, opencontainers/image-spec#819 is now closed via opencontainers/image-spec#1240

@rchincha
Copy link

Does this need rebasing?

github.com/zeebo/assert v1.1.0/go.mod h1:Pq9JiuJQpG8JLJdtkwrJESF0Foym2/D9XMU5ciN/wJ0=
github.com/zeebo/blake3 v0.1.1 h1:Nbsts7DdKThRHHd+YNlqiGlRqGEF2bE2eXN+xQ1hsEs=
github.com/zeebo/blake3 v0.1.1/go.mod h1:G9pM4qQwjRzF1/v7+vabMj/c5mWpGZ2Wzo3Eb4z0pb4=
github.com/zeebo/blake3 v0.2.0 h1:1SGx3IvKWFUU/xl+/7kjdcjjMcvVSm+3dMo/N42afC8=

Choose a reason for hiding this comment

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

now at 0.2.4

rchincha added a commit to rchincha/go-digest that referenced this pull request Apr 25, 2025
opencontainers#66
opencontainers/image-spec#1240
Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com>
@rchincha
Copy link

#108

rchincha added a commit to rchincha/go-digest that referenced this pull request Apr 25, 2025
opencontainers#66
opencontainers/image-spec#1240
Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com>
@rchincha
Copy link

rchincha commented Apr 25, 2025

PR #108 is now passing ci...pls take a look.

rchincha added a commit to project-zot/go-digest that referenced this pull request May 1, 2025
* digest: promote blake3 to first-class digest

The dual module approach for blake3 was slightly awkward. Since it
provides similar usability with a massive bump in performance, it's
extremely likely to land as a registered algorithm in the image-spec.

This PR removes the secondary module, which made it difficult to test as
a unit. This may break users who are using HEAD versions of the package.
For a new release, this will be backwards compatible. The other drawback
is that the zeebo/blake3 will now be a dependency but this can be
replaced transparently by the standard libary in the future.

In addition to promoting blake3, this makes a few style adjustments to
be in line with Go's style guidelines.

Signed-off-by: Stephen Day <stephen.day@getcruise.com>

* fix: update stevvooe's blake3 PR

opencontainers#66
opencontainers/image-spec#1240
Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com>

* fix: add a length test for blake3

Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com>

* fix: add a Makefile

make
make build
make test

Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com>

* fix: merge conflicts

Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com>

* fix: blake3 pulls in golang 1.22.x dep

2025-04-25T19:33:20.2495501Z go: downloading github.com/zeebo/blake3 v0.2.4
2025-04-25T19:33:20.3154128Z go: downloading github.com/klauspost/cpuid/v2 v2.2.10
2025-04-25T19:33:20.3959610Z github.com/klauspost/cpuid/v2: cannot compile Go 1.22 code
2025-04-25T19:33:21.1059807Z FAIL github.com/opencontainers/go-digest [build failed]
2025-04-25T19:33:21.1060449Z FAIL github.com/opencontainers/go-digest/digestset [build failed]
2025-04-25T19:33:21.1219422Z ##[error]Process completed with exit code 1.
Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com>

---------

Signed-off-by: Stephen Day <stephen.day@getcruise.com>
Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com>
Co-authored-by: Stephen Day <stephen.day@getcruise.com>
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.

9 participants