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

Add SHA3 implementation #7

Merged
merged 3 commits into from
Mar 4, 2025
Merged

Add SHA3 implementation #7

merged 3 commits into from
Mar 4, 2025

Conversation

solonovamax
Copy link
Contributor

Adds a SHA3 implementation.

I currently just have

object SHA3_224 : HasherFactory("SHA3-512", { SHA3(224 / Byte.SIZE_BITS) })
object SHA3_256 : HasherFactory("SHA3-256", { SHA3(256 / Byte.SIZE_BITS) })
object SHA3_384 : HasherFactory("SHA3-384", { SHA3(384 / Byte.SIZE_BITS) })
object SHA3_512 : HasherFactory("SHA3-512", { SHA3(512 / Byte.SIZE_BITS) })

Is there perhaps a better way that these could be named?

Also, I chose to just make them objects instead of doing

class SHA3_256 {
    companion object : HasherFactory("SHA3-512", { SHA3(224 / Byte.SIZE_BITS) })
}

Signed-off-by: solonovamax <solonovamax@12oclockpoint.com>
Signed-off-by: solonovamax <solonovamax@12oclockpoint.com>
Signed-off-by: solonovamax <solonovamax@12oclockpoint.com>
Copy link

codecov bot commented Mar 4, 2025

Codecov Report

Attention: Patch coverage is 92.95775% with 5 lines in your changes missing coverage. Please review.

Project coverage is 86.28%. Comparing base (3919b63) to head (0880057).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
korlibs-crypto/src/korlibs/crypto/SHA3.kt 92.95% 4 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main       #7      +/-   ##
==========================================
+ Coverage   85.45%   86.28%   +0.82%     
==========================================
  Files          16       17       +1     
  Lines         770      853      +83     
  Branches       98      109      +11     
==========================================
+ Hits          658      736      +78     
- Misses         89       93       +4     
- Partials       23       24       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@soywiz soywiz requested a review from Copilot March 4, 2025 11:47

Choose a reason for hiding this comment

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

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

@soywiz soywiz merged commit 3c6763e into korlibs:main Mar 4, 2025
6 checks passed
@soywiz
Copy link
Member

soywiz commented Mar 4, 2025

Thanks!

@solonovamax
Copy link
Contributor Author

Thanks!

I know you merged it, but any better ideas for the names of the sha3 functions, or no? because having names with an underscore feels kinda icky, though I'm not sure if there's a better alternative.

although, I guess something like

class SHA3 internal constructor(digestSize: Int) : Hasher(
    chunkSize = 200 - 2 * digestSize,
    digestSize = digestSize,
    name = "SHA3-${digestSize * 8}"
) {

    // ...

    object Digest224 : HasherFactory("SHA3-512", { SHA3(224 / Byte.SIZE_BITS) })
    object Digest256 : HasherFactory("SHA3-256", { SHA3(256 / Byte.SIZE_BITS) })
    object Digest384 : HasherFactory("SHA3-384", { SHA3(384 / Byte.SIZE_BITS) })
    object Digest512 : HasherFactory("SHA3-512", { SHA3(512 / Byte.SIZE_BITS) })
}

could work, however it does not match the existing naming conventions and I'm unsure if it's any better.

(also, I did only just realize that SHA3 doesn't need to be open)

Comment on lines +255 to +256
public static final field KECCAKF_ROUNDS I
public static final field SHA3_KECCAK_SPONGE_WORDS I
Copy link
Member

Choose a reason for hiding this comment

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

private

Comment on lines +264 to +266
public final fun getKECCAKF_PI_LANE ()[I
public final fun getKECCAKF_ROTATION_OFFSETS ()[I
public final fun getKECCAKF_ROUND_CONSTANTS-Y2RjT0g ()[J
Copy link
Member

Choose a reason for hiding this comment

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

private

@@ -250,6 +250,45 @@ public final class korlibs/crypto/SHA256Kt {
public static final fun sha256 ([B)Lkorlibs/crypto/Hash;
}

public class korlibs/crypto/SHA3 : korlibs/crypto/Hasher {
Copy link
Member

Choose a reason for hiding this comment

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

Make it final

Comment on lines +250 to +251
public static final field KECCAKF_ROUNDS I
public static final field SHA3_KECCAK_SPONGE_WORDS I
Copy link
Member

Choose a reason for hiding this comment

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

private

Comment on lines +259 to +261
public final fun getKECCAKF_PI_LANE ()[I
public final fun getKECCAKF_ROTATION_OFFSETS ()[I
public final fun getKECCAKF_ROUND_CONSTANTS-Y2RjT0g ()[J
Copy link
Member

Choose a reason for hiding this comment

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

private

@soywiz
Copy link
Member

soywiz commented Mar 4, 2025

@solonovamax Feel free to make another PR.

Regarding names I'm okay with the current ones.

There is a mistake here:

⚠️ object SHA3_224 : HasherFactory("SHA3-512", { SHA3(224 / Byte.SIZE_BITS) })
->
object SHA3_224 : HasherFactory("SHA3-224", { SHA3(224 / Byte.SIZE_BITS) })

You can remove the open from the class, and also please make private the constants in the companion so they are not exposed.

@solonovamax
Copy link
Contributor Author

oops, must have missed that when copying things around.

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.

2 participants