Skip to content

[frontend] Add public key hash tweak#879

Closed
paulcadman wants to merge 1 commit into08-22-_frontend_add_merkle_tree_verifierfrom
public_key_hash_tweak
Closed

[frontend] Add public key hash tweak#879
paulcadman wants to merge 1 commit into08-22-_frontend_add_merkle_tree_verifierfrom
public_key_hash_tweak

Conversation

@paulcadman
Copy link
Copy Markdown
Contributor

@paulcadman paulcadman commented Aug 31, 2025

This PR adds a Keccak256 tweak circuit that will be used to hash the public keys in the hash-based signature circuit.

Helper functions to compute the hash for tree and chain tweaks are also added.

Copy link
Copy Markdown
Contributor Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more


How to use the Graphite Merge Queue

Add the label merge-ready to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

num_hashes in 1usize..=10,
) {
use rand::SeedableRng;
use rand::prelude::StdRng;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The import use rand::prelude::StdRng is incorrect for reproducible testing. The prelude import does not guarantee availability of SeedableRng trait methods required for StdRng::seed_from_u64. Change to use rand::{SeedableRng, rngs::StdRng}; to ensure proper access to seeding functionality.

Suggested change
use rand::prelude::StdRng;
use rand::{SeedableRng, rngs::StdRng};

Spotted by Diamond (based on custom rule: Monbijou Testing Patterns)

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

@paulcadman paulcadman force-pushed the 08-22-_frontend_add_merkle_tree_verifier branch from 73076a9 to 9275d23 Compare September 1, 2025 00:13
@paulcadman paulcadman force-pushed the public_key_hash_tweak branch 2 times, most recently from bb2c6a3 to fcc32e5 Compare September 1, 2025 08:12
@paulcadman paulcadman force-pushed the 08-22-_frontend_add_merkle_tree_verifier branch from 9275d23 to 50b3a36 Compare September 1, 2025 08:12
@paulcadman paulcadman marked this pull request as ready for review September 1, 2025 08:21
@paulcadman paulcadman force-pushed the public_key_hash_tweak branch from fcc32e5 to b9b2273 Compare September 1, 2025 08:39
@paulcadman paulcadman force-pushed the 08-22-_frontend_add_merkle_tree_verifier branch from 50b3a36 to db4c440 Compare September 1, 2025 08:39
Comment on lines +332 to +334
use rand::SeedableRng;
use rand::prelude::StdRng;

let mut rng = StdRng::seed_from_u64(0);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Property-based tests should not use seeded random number generators. This test violates the Pseudo-Random Testing rule by importing rand::SeedableRng and StdRng within the proptest! macro and using StdRng::seed_from_u64(0) for deterministic random generation. Property-based tests using the proptest crate should rely on rand::rng() or proptest's built-in randomization strategies instead of seeded RNG. Seeded random generation with StdRng::seed_from_u64 is appropriate for reproducible unit tests but not for property-based tests. Remove the rand imports and StdRng usage, and allow proptest to handle randomization internally.

Spotted by Diamond (based on custom rule: Monbijou Testing Patterns)

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

@paulcadman paulcadman requested a review from jadnohra September 1, 2025 08:41
@paulcadman paulcadman force-pushed the 08-22-_frontend_add_merkle_tree_verifier branch from db4c440 to c77be67 Compare September 1, 2025 12:08
@paulcadman paulcadman force-pushed the public_key_hash_tweak branch from b9b2273 to 017db1b Compare September 1, 2025 12:08
@paulcadman paulcadman force-pushed the 08-22-_frontend_add_merkle_tree_verifier branch from c77be67 to a07698b Compare September 1, 2025 15:05
@paulcadman paulcadman force-pushed the public_key_hash_tweak branch from 017db1b to 84124c1 Compare September 1, 2025 15:05
@graphite-app
Copy link
Copy Markdown

graphite-app bot commented Sep 1, 2025

Merge activity

  • Sep 1, 3:16 PM UTC: paulcadman added this pull request to the Graphite merge queue.
  • Sep 1, 3:16 PM UTC: CI is running for this pull request on a draft pull request (#897) due to your merge queue CI optimization settings.
  • Sep 1, 3:22 PM UTC: Merged by the Graphite merge queue via draft PR: #897.

graphite-app bot pushed a commit that referenced this pull request Sep 1, 2025
This PR adds a Keccak256 tweak circuit that will be used to hash the public keys in the hash-based signature circuit.

Helper functions to compute the hash for tree and chain tweaks are also added.
@graphite-app graphite-app bot closed this Sep 1, 2025
@graphite-app graphite-app bot deleted the public_key_hash_tweak branch September 1, 2025 15:22
lockedloop pushed a commit that referenced this pull request Sep 8, 2025
This PR adds a Keccak256 tweak circuit that will be used to hash the public keys in the hash-based signature circuit.

Helper functions to compute the hash for tree and chain tweaks are also added.
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