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

feat: Add Blake2s benchmark program #241

Closed
wants to merge 1 commit into from
Closed

Conversation

storojs72
Copy link
Member

Adds simple Sphinx program that performs single blake2s hashing, useful for testing and benchmarking the correspondent precompile (argumentcomputer/sphinx#176).

The integration code is located here.

This PR is better to merge after argumentcomputer/sphinx#176 with correspondent update to Cargo.toml.

Nit: In order to compile the programs (run make), it is necessary to downgrade hybrid-array dependency in every program in programs and also in the kadena workspace (hardcoding 0.2.0-rc.9 version in Cargo.toml didn't help me to workaround this):

cargo update [email protected] --precise 0.2.0-rc.9

Copy link
Contributor

Choose a reason for hiding this comment

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

An update of the lock file can make sense but a deletion might be a tad too violent 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, that was accidentally. Returned it back

Copy link
Contributor

@wwared wwared left a comment

Choose a reason for hiding this comment

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

I think the code in this PR is good to merge, except we need to merge argumentcomputer/sphinx#176 first and point the Cargo.toml back at dev instead of artem/blake2s

[patch.crates-io]
# Sphinx patch
#tiny-keccak = { git = "https://github.com/sp1-patches/tiny-keccak", branch = "patch-v2.0.2" }
sha2 = { git = "https://github.com/sp1-patches/RustCrypto-hashes", branch = "patch-v0.10.8" }
Copy link
Contributor

@wwared wwared Sep 27, 2024

Choose a reason for hiding this comment

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

I think we need to keep this [patch.crates-io] section, though we can point it at our own fork (zkvm branch) instead of the sp1-patches one

[patch.crates-io]
# Sphinx patch
#tiny-keccak = { git = "https://github.com/sp1-patches/tiny-keccak", branch = "patch-v2.0.2" }
sha2 = { git = "https://github.com/sp1-patches/RustCrypto-hashes", branch = "patch-v0.10.8" }
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

@storojs72
Copy link
Member Author

On hold for a while as it requires new sphinx release

@storojs72
Copy link
Member Author

Closing in favour of #256

@storojs72 storojs72 closed this Sep 30, 2024
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