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

perf(merkle): optimize Poseidon hash using direct hades_permutation #1280

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

0x471
Copy link

@0x471 0x471 commented Jan 1, 2025

Fixes #1268

PR Checklist

  • Tests
  • Documentation
  • Added entry to CHANGELOG.md
  • Tried the feature on a public network

Copy link
Member

@ericnordelo ericnordelo left a comment

Choose a reason for hiding this comment

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

Hey, the update looks good. You need to run scarb fmt -w for the linter, and some tests need to be fixed as well, which can be more time-consuming. Note that when the PR is ready, it still may take some time to get merged, since we also need to update https://github.com/ericnordelo/strk-merkle-tree to make it compatible with the new format.

@@ -33,14 +34,15 @@ pub impl PedersenCHasher of CommutativeHasher {

/// Computes the Poseidon commutative hash of a sorted pair of felt252 values.
pub impl PoseidonCHasher of CommutativeHasher {
/// Computes the Poseidon hash of the concatenation of two values, sorting the pair first.
/// Computes the Poseidon hash by directly using hades_permutation with the two values
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// Computes the Poseidon hash by directly using hades_permutation with the two values
/// Computes the Poseidon hash of two values, sorting the pair first.

@0x471
Copy link
Author

0x471 commented Jan 4, 2025

Thanks for the review, @ericnordelo . Feel free to ping me once the other dependencies are ready. I’d be happy to finalize and prepare it for merge.

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.

Reduce the number of permutations in merkle tree leave hashing
2 participants