Adds property-based testing and fuzzing for SMT

[krushimir]

This PR introduces fuzz testing for the Sparse Merkle Tree (Smt) implementation.
It adds fuzzing targets, property-based tests, ensuring correctness and consistency between sequential and concurrent implementations.

[gswirski]

The code looks good to me. It's probably expected, but heads up that tests for merkle::smt::full::concurrent now take 5 min on my M3 Max processor.

[bobbinth]

Looks good! Thank you! I left a few minor comments inline. A couple of other questions/comments:

In this PR we create the miden-crypto-fuzz crate which is in the directory tree of the miden-crypto crate - would this affect publishing of the miden-crypto crate somehow (e.g., would the fuzz crate get included with the crypto crate when we publish)? Maybe it makes sense to put them in separate directories - e.g., something like:

.
├── miden-crypto
│   ├── scr
│   ├── Cargo.toml
│   └── README.md
├── miden-crypto-fuzz
│   ├── src
│   ├── Cargo.toml
│   └── README.md
├── Cargo.toml
├── CHANGELOG.md
├── README.md
├── Makefile
└── etc.

The code looks good to me. It's probably expected, but heads up that tests for merkle::smt::full::concurrent now take 5 min on my M3 Max processor.

I'm not a CI expert, but is it possible to run these tests only when merging into next or main? Maybe we could mark them as "ignored" and then use different commands for when merging into next or main vs. pushing a commit or merging into other branches. cc @Mirko-von-Leipzig.

[Mirko-von-Leipzig]

I'm not a CI expert, but is it possible to run these tests only when merging into next or main? Maybe we could mark them as "ignored" and then use different commands for when merging into next or main vs. pushing a commit or merging into other branches. cc @Mirko-von-Leipzig.

Any of these is possible; one just needs to decide precisely when one wants them to run.

1. Post merge into main/next
2. Any PR change when PR targets main/next
3. Manually via a command / label

The simplest is having a separate job/step which checks what the github target ref is. If its main/next then it should be active, else skipped. If you want to save on compilation time or caching then having it be an optional step within the test job makes sense to me. This is assuming the compilation remains the same.

---

There are also several ways of segregating the tests:

1. With a feature flag (seems like a poor choice here, they should always compile I imagine).
2. Using #[ignore], this works.
3. Target them for exclusion via name/path.
4. Make them a separate binary / integration-test, or use cargo xtask

An issue with (2) is if you truly do want to ignore a test, you now cannot do this because you're (ab)using ignore to mean expensive test (and they do get run on occasion).

(3) means having them on a common path prefix, or sharing a common test name prefix so you can exclude them using the test runner. The downside is that you can no longer trivially cargo test without including these massive tests. If you're using nextest then they have a whole section on how to configure this by default.

I would use (3) myself I think.

[krushimir]

In this PR we create the miden-crypto-fuzz crate which is in the directory tree of the miden-crypto crate - would this affect publishing of the miden-crypto crate somehow (e.g., would the fuzz crate get included with the crypto crate when we publish)?

I verified that miden-crypto-fuzz is not included in the published crate:

• publish = false is set in fuzz/Cargo.toml, ensuring it won't be published.
• cargo package --list confirmed that fuzz/ is excluded.

Cargo treats it as a separate crate and does not include it in the main package.

For extra safety, I can restructure the tree as suggested, or explicitly add exclude = ["fuzz/"] to Cargo.toml.

[bobbinth]

Looks good! Thank you! Would also be good to get a review from @PhilippGackstatter before merging.

How difficult would it be to update the CI to run the expensive tests only when merging to main/next? If not too difficult, I'd do it in this PR - otherwise, we can leave this for a follow up.

I verified that miden-crypto-fuzz is not included in the published crate:

• publish = false is set in fuzz/Cargo.toml, ensuring it won't be published.
• cargo package --list confirmed that fuzz/ is excluded.

Cargo treats it as a separate crate and does not include it in the main package.

For extra safety, I can restructure the tree as suggested, or explicitly add exclude = ["fuzz/"] to Cargo.toml.

I would still probably update the directory structure, but let's do it in a follow-up PR.

[Mirko-von-Leipzig]

Looks good! Thank you! Would also be good to get a review from @PhilippGackstatter before merging.

How difficult would it be to update the CI to run the expensive tests only when merging to main/next? If not too difficult, I'd do it in this PR - otherwise, we can leave this for a follow up.

Just to confirm: merging into main/next means the tests will only run after the merge has already occurred. Or do you want it to happen on PR activity into main/next?

In either case it will require slightly changing these lines:

crypto/.github/workflows/test.yml

Lines 25 to 28 in 8ce7b68

	- name: Perform tests
	run: |
	rustup update --no-self-update ${{matrix.toolchain}}
	make test-${{matrix.args}}

into something like:

      - name: Update rust
        run: rustup update --no-self-update ${{matrix.toolchain}}
      - name: Run tests (cheap)
        run: make test-${{matrix.args}}
      - name: Run expensive tests on PR into main/next
        if: ${{ github.event_name == 'pull_request' && (github.base_ref == 'main' || github.base_ref == 'next') }} 
        run: make expensive-test
      - name: Run expensive tests on merge into main/next
        if: ${{ github.event_name == 'push' && (github.ref == 'main' || github.ref == 'next') }}
        run: make expensive-test

[krushimir]

Thanks @Mirko-von-Leipzig!
I think it's best to run these tests before merging to catch issues early. I'll update the workflow to run pre-merge for now.
cc @bobbinth in case you have more thoughts on this.

[gswirski]

One more nit: fuzz/Cargo.lock is not committed so running fuzzing creates changes in my working directory.

[PhilippGackstatter]

Looks good to me!

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[krushimir]

@Mirko-von-Leipzig, to avoid redundant runs in each matrix combination, I moved the expensive tests into a separate job that runs only once per toolchain (stable & nightly) on PRs targeting either main or next. Let me know if that works for you!