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

Cleaned up sha256.rs and added test cases #610

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

Conversation

joeyschmoe
Copy link

@joeyschmoe joeyschmoe commented Sep 10, 2024

This PR updates the SHA-256 hash function with a fixed-size 32-byte array and adds test cases.

In this version:
The copy_from_slice method is used to convert the hash result (which is always a 32-byte digest) into a fixed-size array. This method is safe and avoids using unwrap() or dealing with Result types for conversions. unwrap() is unsafe because it risks task failure and thus is not a composable error handling mechanism.
SHA-256 always produces a 32-byte digest, so an assertion to check the output length was redundant. Removing it simplifies error handling.

  • There was only one test case for the hash, so I added two new test cases, one for b"hello world" and one for b"", to verify consistency.

Edit:
Error handling was unnecessary, so this change now only adds test cases for the hash.

@CLAassistant
Copy link

CLAassistant commented Sep 10, 2024

CLA assistant check
All committers have signed the CLA.

crates/crypto/src/hashes/sha256.rs Outdated Show resolved Hide resolved
crates/crypto/src/hashes/sha256.rs Outdated Show resolved Hide resolved
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