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

Replacing Blake2s compression function by Blake2s hash function #190

Merged
merged 9 commits into from
Apr 22, 2020

Conversation

rrtoledo
Copy link
Contributor

@rrtoledo rrtoledo commented Apr 8, 2020

Tackles #158

libzeth/circuits/blake2s/blake2s_comp.hpp Outdated Show resolved Hide resolved
libzeth/circuits/blake2s/blake2s_comp.hpp Outdated Show resolved Hide resolved
libzeth/circuits/blake2s/blake2s.tcc Outdated Show resolved Hide resolved
libzeth/circuits/blake2s/blake2s.tcc Outdated Show resolved Hide resolved
size_t to_pad = input_size % BLAKE2s_block_size;
for (size_t i = 0; i < BLAKE2s_block_size - to_pad; i++) {
padded_input.push_back(FieldT("0"));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like here you will still pad with 0's even if input_size % BLAKE2s_block_size == 0. In fact the result of your mod is stored in to_pad, and to_pad is not used inside a conditional to pad only if to_pad != 0 as suggested in your comment above. If to_pad is 0 here - assuming input_size = BLAKE2s_block_size, then your padded_input vector will be of length 2*BLAKE2s_block_size.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

libzeth/circuits/blake2s/blake2s_comp_setup.tcc Outdated Show resolved Hide resolved
libzeth/test/blake2s_test.cpp Outdated Show resolved Hide resolved
@AntoineRondelet
Copy link
Contributor

@rrtoledo please make sure to rebase your branch on top of develop since the build has been repaired.

@dtebbs dtebbs force-pushed the blake-hash branch 2 times, most recently from 45e00a1 to 5037687 Compare April 21, 2020 18:03
Copy link
Contributor

@dtebbs dtebbs left a comment

Choose a reason for hiding this comment

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

Rebased onto develop and fixed a few build issues.

@@ -165,6 +165,7 @@ template<typename FieldT> class BLAKE2s_256 : public libsnark::gadget<FieldT>
};

} // namespace libzeth

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure this is consistent with the other hpp/tcc files where we wrote the include on the line following the } closing the libzeh namespace

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I didn't realise it had been switched to that.
It seems slightly unnatural since in all other cases we have a line before #include blocks, and line between } // namespace ... and following code (hence it stood out).
Anyway changed for now to be consistent.

@AntoineRondelet AntoineRondelet merged commit 6c7c694 into develop Apr 22, 2020
AntoineRondelet added a commit that referenced this pull request May 6, 2020
Replacing Blake2s compression function by Blake2s hash function
@AntoineRondelet AntoineRondelet deleted the blake-hash branch July 28, 2020 17:53
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.

3 participants