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

[Feature]: Add Differential Tests for _keccak.rs_ #503

Open
1 task done
0xNeshi opened this issue Jan 21, 2025 · 0 comments
Open
1 task done

[Feature]: Add Differential Tests for _keccak.rs_ #503

0xNeshi opened this issue Jan 21, 2025 · 0 comments
Labels
effort: medium Default level of effort. priority: 0 Nice-to-have. Willing to ship without this. type: test Changes to the testing suite.

Comments

@0xNeshi
Copy link
Collaborator

0xNeshi commented Jan 21, 2025

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 keccak logic implementation against some third-party implementation. This would help pinpoint any latent issues, bugs etc.

One possible candidate for comparison is keccak.

Note

Even though this is just a wrapper around tiny-keccak, considering the fact this library has very few tests, it's worth adding tests on our side.
Alternatively, one can create a PR adding tests to tiny-keccak itself, which should accomplish the same thing. This is arguably less optimal, because in the case we decide to switch implementations in the future to some other keccak crate, we'd still have our existing testing suite.

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
3 tasks
@bidzyyys bidzyyys added this to the Release v0.2.0 milestone Feb 4, 2025
0xNeshi added a commit that referenced this issue Feb 5, 2025
<!--
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
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
Status: Todo
Development

No branches or pull requests

2 participants