feat(stm): Implementation of the digest trait for the Poseidon hash from Midnight

[damrobi]

Content

This PR includes a wrapper that implements the Digest trait to be used with the Poseidon hash function from Midnight.

Pre-submit checklist

• Branch
  • Tests are provided (if possible)
  • Crates versions are updated (if relevant)
  • CHANGELOG file is updated (if relevant)
  • Commit sequence broadly makes sense
  • Key commits have useful messages
• PR
  • All check jobs of the CI have succeeded
  • Self-reviewed the diff
  • Useful pull request description
  • Reviewer requested
• Documentation
  • No new TODOs introduced

Comments

Quick summary of the important traits implemented:

• Update: updates the internal buffer with new data, takes a slice of bytes as input.
• FixedOutput: contains the finalize function that outputs the result of the hash, since Poseidon takes base field elements as input there is a conversion from the bytes of the buffer to JubjubBase elements before applying the hash.

I have a question regarding the design of the digest functions:

• I made it so the update function always pad the data received to a multiple of 32 bytes to make sure we can generate an exact number of JubjubBase elements when computing the hash. This padding can be moved to the finalize function. I did it this way because I wasn't sure how we wanted to handle the case where only one byte is received by the hasher at a time.
• Example: We feed one byte to the hasher three times: 1u8, 2u8, 3u8. Do we want this to correspond to applying poseidon on three JubjubBase elements Poseidon(F(1), F(2), F(3)) or to one element create from those bytes Poseidon(F(b"0000 0001 0000 0010 0000 0011"))?
• The current implementation correspond to the first case but it can be changed

Last remark, this problem concerns the CPU implementation of Poseidon. In the circuit, we only deal with JubjubBase elements so it is clear what is done. We need to make sure both implementations give the same results.

I think that given the current use of digest in the implementation, it does not matter too much which way we use as the example I gave does not come up but I wanted to check with more people as it might change in the future.

Issue(s)

Relates to #2942

[github-actions]

Test Results

    5 files  ± 0    172 suites  ±0   30m 52s ⏱️ + 1m 31s
2 421 tests +17  2 421 ✅ +17  0 💤 ±0  0 ❌ ±0 
7 533 runs  +17  7 533 ✅ +17  0 💤 ±0  0 ❌ ±0 

Results for commit b58a880. ± Comparison against base commit 216ee9f.

♻️ This comment has been updated with latest results.

[damrobi]

I'm not sure the poseidon_digest.rs file is the correct place for implementing those traits but I don't really know where to put them for now so let me know if you have an idea.

[hjeljeli32]

I'm not sure the poseidon_digest.rs file is the correct place for implementing those traits but I don't really know where to put them for now so let me know if you have an idea.

@damrobi
IMO Since this is a generic hashing adapter (likely used by Merkle tree / membership commitment and potentially elsewhere), I don’t think it should live under signature_scheme/unique_schnorr_signature/....
I’d suggest moving it to a more neutral place like mithril-stm/src/hash/poseidon.rs

[rrtoledo]

The current implementation correspond to the first case but it can be changed

To me the first case would make more sense as you are concatenating 3 different things. If you want to do the second case, the user could concatenate the bytes first in a buffer then hash this one instead.

Let's have a look about this later comparing it with Midnight's implementation too.

[Copilot]

Pull request overview

This PR implements a Digest trait wrapper for the Poseidon hash function from Midnight, enabling its use in SNARK pre-aggregation primitives for the Mithril STM library. The implementation allows the Poseidon hash to be used as the SnarkHash type in Merkle tree constructions.

Changes:

• Added MidnightPoseidonDigest struct implementing the Digest trait (Update, FixedOutput, Reset, HashMarker, OutputSizeUser)
• Updated MithrilMembershipDigest::SnarkHash to use MidnightPoseidonDigest instead of the Blake2b placeholder
• Added comprehensive tests for the digest implementation including edge cases
• Added Merkle tree tests using the new Poseidon digest with the future_snark feature

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 13 comments.

File	Description
mithril-stm/src/signature_scheme/unique_schnorr_signature/jubjub/poseidon_digest.rs	Implements MidnightPoseidonDigest wrapper struct with Digest trait implementation and test suite
mithril-stm/src/lib.rs	Updates SnarkHash type alias to use MidnightPoseidonDigest and adds necessary imports
mithril-stm/src/membership_commitment/merkle_tree/tree.rs	Adds comprehensive property-based tests for Merkle tree operations using Poseidon digest

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[rrtoledo]

We have a hash function that in circuit takes as input &[Fq] but in CPU takes &[u8] (to satisfy the trait's interfaces). Hence we need to convert somewhere, when updating or finalizing the hasher, the bytes to field elements.

As we work with prime fields, we cannot directly cast the bytes to field elements. We need to either take fewer bytes than our field's prime and cast, take 32 bytes and check we are under the modulus before casting, or take more bytes and somehow hash them to the field.

p.s. I am trying to be careful saying "somehow hash to field" here and from now on because we may not choose to use the RFC's function hash_to_field for efficiency. We could simply hash with sha256/blake2 and take the modulo of it with a small loss in soundness instead.

In @kitounliu 's circuit, we always work with field element inputs:

• the Schnorr signature's message is a field element in circuit - so it has already been converted somewhen
• the Merkle tree's leaves' data
  -- vks which are group element of Jubjub hence scalar of Bls12-381
  -- the target which should be small (64 bit iirc) and can be directly cast as a field

In this code, we could assume that the hasher's &[u8] input already are Fq's byte representation. In that case, we should add some comments on the Poseidon code stating it (perhaps some assert?) and in registration/signing adding the extra conversion (the target must be cast to Fq, and message hashed to Fq).
---> I think that's fine as long as it is well documented but I would like your opinion.

If we don't want to assume that the hasher's &[u8] input already are Fq's byte representations., for instance for the hash function to be generic, the design become a bit delicate.
First, do we want the property that hasher.update(a).update(b).finalize() == hasher.update(a || b).finalize()?

• if yes, we are forced to concatenate new bytes to the state and later on cast the state to scalar elements.

1. Taking less than 32 bytes when converting to Fq would not be compatible with @kitounliu 's code in circuit.
2. Taking 32 bytes exactly can be compatible if we do a modulo check and somehow hash to field only when needed
3. Taking more than 32 bytes at a time would not be compatible with @kitounliu 's code in circuit.
4. For the remainder (less than 32 bytes), we can (pad and) cast or hash

• if not necessarily,

1. we can pad each update's input. In that case, the padding must be consistent with @kitounliu 's code in circuit.
2. convert each update's input to field element. We have here the same question as before of how many bytes we take at at time.
3. other solutions?

Second, depending on the choice choice above, we may want to restrict the hasher so that the update function takes a "fixed" number of bytes (e.g. at most 32 bytes or a multiple of 32 bytes). We can ensure that with asserts.

Maybe I am overthinking, or I missed something out, but we should all agree on what design choice to take.

[hjeljeli32]

LGTM. I just left a few comments/suggestions.

[hjeljeli32]

LGTM. I just left a few comments/suggestions.

All my questions were answered by @damrobi

[hjeljeli32]

LGTM.

[curiecrypt]

LGTM 👍

Regarding the digest implementation, I don’t see any issues or inconsistencies for the use cases covered in this branch. That said, it’s still hard to be certain whether any problems might occur when we integrate this into other parts of the codebase, we’ll be able to assess that more clearly as we progress.
The documentation looks sufficient and well explained. Thanks for putting in the work on that @rrtoledo, @damrobi .

One concern I do have is the direct use of Midnight library entities in this module (JubjubBase, from_raw, etc). We should add wrappers or use the existing ones for those occurrences. This doesn’t necessarily need to be done in this PR, but it’s something we should plan to do in the near future.

[jpraynaud]

LGTM