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

Migrate hashing from blake2b to blake3. #85

Merged
merged 1 commit into from May 22, 2023
Merged

Conversation

jsantell
Copy link
Contributor

We're considering using blake3 for our hashing in noosphere using it with Ucan. Changing the hashing would be a breaking change, not matching previous encodings. Similarly, we could make the hashing generic. What do you all think?

@jsantell jsantell requested review from cdata and a team as code owners May 18, 2023 23:43
@expede
Copy link
Member

expede commented May 18, 2023

FWIW: at the spec level, there's nothing preventing you from using BLAKE3. There is a question about canonicalization for revocation, but the links inside the UCAN itself are agnostic to which hash they use as long as they're resolvable

(Also IMO BLAKE3 is awesome, and we're starting to see it show up a buch in the ecosystem)

@jsantell
Copy link
Contributor Author

More specifically, rs-ucan worked flawlessly when trying out blake3 other than ProofChain::from_ucan, which I think is calling the blake2b-specific Cid::try_from(&Ucan) somewhere

@cdata
Copy link
Member

cdata commented May 19, 2023

@jsantell would you be willing to clean up the lint checks?

@jsantell
Copy link
Contributor Author

jsantell commented May 19, 2023

@jsantell would you be willing to clean up the lint checks?

Updated! Edit: one lint failed on nightly, repushing

@jsantell jsantell force-pushed the blake3 branch 2 times, most recently from 7780472 to e51dfb2 Compare May 19, 2023 17:03
@cdata
Copy link
Member

cdata commented May 22, 2023

A rebase will likely make this change pass muster :)

Copy link
Member

@cdata cdata left a comment

Choose a reason for hiding this comment

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

Yay!

@cdata cdata merged commit 205cb96 into ucan-wg:main May 22, 2023
5 checks passed
@github-actions github-actions bot mentioned this pull request May 22, 2023
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