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

Audit preparedness: test binary operations #5426

Closed
20 of 26 tasks
michaeljklein opened this issue Jul 5, 2024 · 0 comments · Fixed by #5484
Closed
20 of 26 tasks

Audit preparedness: test binary operations #5426

michaeljklein opened this issue Jul 5, 2024 · 0 comments · Fixed by #5484
Assignees
Labels
enhancement New feature or request

Comments

@michaeljklein
Copy link
Contributor

michaeljklein commented Jul 5, 2024

Problem

Part of #5362

// "kind" in binary::Binary (op)
pub(crate) enum BinaryOp {
  • Sanity-check that behavior of these match for example inputs 0..8 (or are None): get_field_function, get_u128_function, get_i128_function

Magmas:

  • Test associativity
  • Test (anti-)commutativity (add_sub and sub_add tests implicitly do so)
  • Test identity
  • Add
  • Sub
  • Mul
  • Div
  • Mod
  • And
  • Or
  • Xor
  • Shl
  • Shr

Comparisons:

  • Test (anti-)reflexivity
  • Test transitivity
  • Test (anti-)commutativity
  • Eq
  • Lt
pub enum BlackBoxFunc {
  • AND
  • XOR
  • EmbeddedCurveAdd
  • BigIntAdd
  • BigIntSub
  • BigIntMul
  • BigIntDiv

Happy Case

Test all listed binary operations

Project Impact

None

Impact Context

No response

Workaround

None

Workaround Description

No response

Additional Context

No response

Would you like to submit a PR for this Issue?

None

Support Needs

No response

@michaeljklein michaeljklein added the enhancement New feature or request label Jul 5, 2024
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Jul 5, 2024
@michaeljklein michaeljklein self-assigned this Jul 5, 2024
github-merge-queue bot pushed a commit that referenced this issue Jul 30, 2024
# Description

TODO:
- [x] Split out control flow tests into a separate PR
#5558
- [x] Split out `BigInt` tests into a separate PR
#5559
- [x] Refactor `any::<T>()`
- [x] Make issues for failing tests

## Problem\*

Resolves #5426

## Summary\*



## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir Jul 30, 2024
github-merge-queue bot pushed a commit that referenced this issue Aug 26, 2024
# Description

## Problem\*

Part of #5426

## Summary\*

Tests
- Conversion to/from LE arrays of bytes
- Add/Mul associative/commutative/identity
- Mul has L/R zero
- Distributive property
- Invertibility of Add/Mul
- Injectivity of Add

## Additional Context

Without updating `overflow-checks` defaults, the invalid-modulus only
panics locally while passing in CI.

It results in a 22% increase in user time and 27% increase in wall-clock
time running `cargo test` (in release mode):
- With overflow checks: `708.25s user 38.29s system 842% cpu 1:28.65
total`
- Without overflow checks: `577.09s user 32.65s system 875% cpu 1:09.68
total`

## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant