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

Updating BLAKE-256 page #1034

Merged
merged 1 commit into from
Jan 9, 2020
Merged

Updating BLAKE-256 page #1034

merged 1 commit into from
Jan 9, 2020

Conversation

0xmzz
Copy link
Member

@0xmzz 0xmzz commented Dec 13, 2019

BLAKE-256 page updates. The update is based on various academic sources as well as on NIST reports. I have created some illustrations in SVG to explain some concepts, as well.

closes #947

@0xmzz 0xmzz changed the title Updating BLAKE-256 page closes #947 Updating BLAKE-256 page Dec 13, 2019
@dajohi
Copy link
Member

dajohi commented Dec 16, 2019

@zubairzia0 please rebase over master

Copy link
Member

@jholdstock jholdstock left a comment

Choose a reason for hiding this comment

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

Despite the large number of changes requested, this is really great stuff @zubairzia0, thanks a lot for the contribution!

One more point to add - as we discussed on Matrix, filenames need to have spaces removed.

docs/research/blake-256-hash-function.md Show resolved Hide resolved
docs/research/blake-256-hash-function.md Outdated Show resolved Hide resolved
docs/research/blake-256-hash-function.md Outdated Show resolved Hide resolved
docs/research/blake-256-hash-function.md Outdated Show resolved Hide resolved
docs/research/blake-256-hash-function.md Outdated Show resolved Hide resolved
docs/research/blake-256-hash-function.md Outdated Show resolved Hide resolved
docs/research/blake-256-hash-function.md Outdated Show resolved Hide resolved
docs/research/blake-256-hash-function.md Outdated Show resolved Hide resolved
docs/research/blake-256-hash-function.md Outdated Show resolved Hide resolved
docs/research/blake-256-hash-function.md Outdated Show resolved Hide resolved
@0xmzz
Copy link
Member Author

0xmzz commented Dec 17, 2019

Despite the large number of changes requested, this is really great stuff @zubairzia0, thanks a lot for the contribution!

One more point to add - as we discussed on Matrix, filenames need to have spaces removed.

Very detailed review and I agree with the changes requested, thank you @jholdstock. I believe I got everything requested.

Copy link
Member

@jholdstock jholdstock left a comment

Choose a reason for hiding this comment

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

A couple of extra things noticed.

I believe @davecgh said he would give this a once over as well, so it would be great to get an approval from him before this is merged.

docs/research/blake-256-hash-function.md Outdated Show resolved Hide resolved
docs/research/blake-256-hash-function.md Outdated Show resolved Hide resolved
docs/research/blake-256-hash-function.md Show resolved Hide resolved
docs/research/blake-256-hash-function.md Outdated Show resolved Hide resolved
docs/research/blake-256-hash-function.md Outdated Show resolved Hide resolved
docs/research/blake-256-hash-function.md Outdated Show resolved Hide resolved
docs/research/blake-256-hash-function.md Outdated Show resolved Hide resolved

A more detailed description of the compression function can be found in [SHA-3 proposal BLAKE](https://decred.org/research/aumasson2010.pdf).

#### ChaCha inspired core function
Copy link
Member

Choose a reason for hiding this comment

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

Honestly, I see no benefit to including this in the documentation. It is all well documented by the authors of the hash function themselves in the standard that describes it.

I feel like the purpose of this documentation should be to describe the core concepts involved in informing the decision to use the hash function.

That means the sections on HAIFA vs M-D, security evaluation criteria, security bounds, and software and hardware performance all make sense, but just regurgitating the specification does not, imo.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, I have taken out those sections. How does it look now?

@jholdstock
Copy link
Member

Looking good to me, just need to remove all of the inline html super/sub-scripting

@jholdstock jholdstock merged commit df1b791 into decred:master Jan 9, 2020
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.

Add details BLAKE-256 hash function page
4 participants