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.

Sign up for GitHub

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

Jump to bottom

[Feature]: Add Differential Tests for Functions in _merkle.rs_ #502

Open
1 task done
0xNeshi opened this issue Jan 21, 2025 · 4 comments · May be fixed by #524
Open
1 task done

[Feature]: Add Differential Tests for Functions in _merkle.rs_ #502

0xNeshi opened this issue Jan 21, 2025 · 4 comments · May be fixed by #524
Assignees
@Haxry
Labels
effort: medium Default level of effort. priority: 0 Nice-to-have. Willing to ship without this. type: test Changes to the testing suite.
Milestone
Release v0.2.0

Comments

@0xNeshi
Copy link
Collaborator

0xNeshi commented Jan 21, 2025 •
edited
Loading

What is the feature you would like to see?

Towards #520

See how Solidity's Foundry defines differential testing.

Differential tests would compare the results of our merkle verification implementation against some third-party implementation. This would help pinpoint any latent issues, bugs etc.

One possible candidate for this is rs-merkle, but we can search for other implementations if they're more robust/well-known

Contribution Guidelines

  • I agree to follow this project's Contribution Guidelines
@0xNeshi 0xNeshi added effort: medium Default level of effort. priority: 0 Nice-to-have. Willing to ship without this. type: test Changes to the testing suite. labels Jan 21, 2025
@0xNeshi 0xNeshi mentioned this issue Jan 21, 2025
Merged
3 tasks
@Haxry
Copy link

Haxry commented Feb 1, 2025

Hi @0xNeshi, I’d love to work on this issue. I’ve previously implemented Merkle Trees in Rust and shared my work in this repository. Please assign this issue to me. Thank you!

@bidzyyys
Copy link
Collaborator

bidzyyys commented Feb 2, 2025

Hi @Haxry! Thanks for your interest in contributing to our library! Please let us know if you have any questions.
Before delivering the tests, please provide us overview how you are going to solve this issue.

CC: @0xNeshi

@Haxry
Copy link

Haxry commented Feb 3, 2025 •
edited
Loading

Differential Testing Implementation Plan for Merkle Verification

Proposed Approach 🔍

  1. Generate Test Cases:
    Create diverse test cases to ensure thorough coverage of different scenarios.

  2. Run Through Stylus Implementation:
    Execute the generated test cases using our Stylus-based Merkle proof implementation.

  3. Run Through rs-merkle:
    Execute the same test cases using the rs-merkle library as a reference implementation.

  4. Compare Results:
    Analyze the outputs from both implementations to identify any discrepancies.

  5. Evaluate Outcome:

    • If the results match, the test passes.
    • If the results do not match, flag the discrepancy for further investigation.

I hope the approach is correct. Please suggest any changes or improvements to my approach.

One question I have is: Should I do differential fuzzing or are differential unit tests sufficient?

Thank you!

@0xNeshi
Copy link
Collaborator Author

0xNeshi commented Feb 3, 2025 •
edited
Loading

Hi @Haxry , this seems like the right approach. Once you create the first differential test case, please create a draft PR so we can ensure we have the same understanding of the approach.

Regarding your question, differential fuzz tests would be awesome to include! Note that diff. unit tests have higher priority.

@bidzyyys bidzyyys added this to the Release v0.2.0 milestone Feb 4, 2025
@Haxry Haxry linked a pull request Feb 5, 2025 that will close this issue
Draft
3 tasks
0xNeshi added a commit that referenced this issue Feb 5, 2025
@0xNeshi @bidzyyys @qalisander
test: Investigate Fuzzing (#477)
1c86069 
<!--
Thank you for your interest in contributing to OpenZeppelin!

Consider opening an issue for discussion prior to submitting a PR. New
features will be merged faster if they were first discussed and designed
with the team.

Describe the changes introduced in this pull request. Include any
context necessary for understanding the PR's purpose.
-->

As [per our
agreement](https://openzeppelin.slack.com/archives/C06R5AGFE9G/p1737021738986409),
this PR includes a basic fuzz tests setup and some missing unit/property
tests for `openzeppelin-crypto` crate.

I created a couple of related issues to address missing property tests
(see #501 #502 #503).
Adding property tests for contract will be easily possible after `motsu`
is refactored (see
https://github.com/OpenZeppelin/stylus-test-helpers/pull/14/files#r1919897820)

<!-- Fill in with issue number -->
Resolves #458 

#### PR Checklist

<!--
Before merging the pull request all of the following must be completed.
Feel free to submit a PR or Draft PR even if some items are pending.
Some of the items may not apply.
-->

- [x] Tests
- [x] Documentation
- [ ] Changelog

---------

Co-authored-by: Daniel Bigos <[email protected]>
Co-authored-by: Alisander Qoshqosh <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Haxry Haxry

Labels
effort: medium Default level of effort. priority: 0 Nice-to-have. Willing to ship without this. type: test Changes to the testing suite.
Projects
Rust Contracts Stylus
Status: Todo
Milestone
Release v0.2.0
Development

Successfully merging a pull request may close this issue.

Differential Testing for Merkle.rs Haxry/rust-contracts-stylus
3 participants
@0xNeshi @bidzyyys @Haxry