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

Added generic sha256 #1544

Merged
merged 31 commits into from May 18, 2022
Merged

Added generic sha256 #1544

merged 31 commits into from May 18, 2022

Conversation

Braqzen
Copy link
Contributor

@Braqzen Braqzen commented May 13, 2022

Closes #1511

@Braqzen Braqzen added enhancement New feature or request good first issue Good for newcomers lib: std Standard library labels May 13, 2022
@Braqzen Braqzen self-assigned this May 13, 2022
sway-lib-std/src/hash.sw Outdated Show resolved Hide resolved
sway-lib-std/src/hash.sw Outdated Show resolved Hide resolved
sway-lib-std/src/hash.sw Outdated Show resolved Hide resolved
Co-authored-by: John Adler <adlerjohn@users.noreply.github.com>
@adlerjohn adlerjohn requested a review from otrho May 13, 2022 20:17
sway-lib-std/src/hash.sw Outdated Show resolved Hide resolved
@nfurfaro
Copy link
Contributor

The implementation looks good :)
I'd like to see some tests for hashing different types added to the stdlib's test_projects/hashing before merging though.

sway-lib-std/src/hash.sw Outdated Show resolved Hide resolved
@Braqzen
Copy link
Contributor Author

Braqzen commented May 13, 2022

The implementation looks good :) I'd like to see some tests for hashing different types added to the stdlib's test_projects/hashing before merging though.

Sure, I already have some assertions that can be compared to hash_u64. I don't know what a good test would be for the generic types of structs / enums. In my testbed I have the digest and output to compare against but I cannot actually tell if they are correct

@adlerjohn
Copy link
Contributor

The unfortunate only way to test hashing is to go on some online hashing site and put in a few values, then use those values as test vectors.

sway-lib-std/src/hash.sw Outdated Show resolved Hide resolved
@mohammadfawaz
Copy link
Contributor

mohammadfawaz commented May 14, 2022

The unfortunate only way to test hashing is to go on some online hashing site and put in a few values, then use those values as test vectors.

Can we not use a hashing library such as https://docs.rs/sha2/latest/sha2/ in the hashing test? That's what I was hoping we'd accomplish for #1492, and this PR seems like the perfect place to knock that issue out.

Also, we can now deprecate (or even remove) hash_u64 and hash_value.

@adlerjohn
Copy link
Contributor

Can we not use a hashing library such as

Ah, I meant if you wanted to test using only a Sway script and no Rust harness.

@Braqzen Braqzen marked this pull request as draft May 17, 2022 16:01
@mohammadfawaz mohammadfawaz mentioned this pull request May 18, 2022
8 tasks
@Braqzen Braqzen marked this pull request as ready for review May 18, 2022 17:33
@Braqzen Braqzen requested a review from otrho May 18, 2022 17:33
Copy link
Contributor

@mohammadfawaz mohammadfawaz left a comment

Choose a reason for hiding this comment

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

Excellent PR. I appreciate the thorough testing!

examples/hashing/src/main.sw Outdated Show resolved Hide resolved
examples/hashing/src/main.sw Show resolved Hide resolved
@mohammadfawaz
Copy link
Contributor

LGTM! I'd like another set of eyes to review this and I'll approve after.

Copy link
Contributor

@adlerjohn adlerjohn left a comment

Choose a reason for hiding this comment

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

nit

sway-lib-std/src/hash.sw Show resolved Hide resolved
sway-lib-std/src/hash.sw Outdated Show resolved Hide resolved
Co-authored-by: John Adler <adlerjohn@users.noreply.github.com>
@Braqzen Braqzen merged commit 8e3c174 into master May 18, 2022
@Braqzen Braqzen deleted the braqzen-1511 branch May 18, 2022 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers lib: std Standard library
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Generic stack type hashing
5 participants