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

refactor: rm math package dependencies #325

Merged
merged 1 commit into from
Aug 21, 2024

Conversation

milancermak
Copy link
Contributor

Pull Request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build-related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

The math package has a dependency on data_structures package only because the ed25519.cairo module uses 3 trivial functions from the SpanTraitExt trait. However, data_structures has a rather wild dependency tree by itself, so it pulls in encoding, numeric, bytes and searching 💀

What is the new behavior?

This PR removes the dependency of the math package on data_structures (and hence all its downstream deps) by adding (or rather, copy-pasting) two private functions (dedup and reverse) to the ed25519.cairo and replacing the third one (concat) by the use of append_span from Cairo core.

Does this introduce a breaking change?

  • Yes
  • No

Other information

There's no regression in performance of the ed25519.

Before this change:

$ scarb test -f ed255
running 7 tests
test alexandria_math::tests::ed25519_test::affine_point_op ... ok (gas usage est.: 411440)
test alexandria_math::tests::ed25519_test::verify_signature_invalid_length ... ok (gas usage est.: 5284792)
test alexandria_math::tests::ed25519_test::verify_signature_test_2 ... ok (gas usage est.: 390532238)
test alexandria_math::tests::ed25519_test::verify_signature_test_1 ... ok (gas usage est.: 382719878)
test alexandria_math::tests::ed25519_test::verify_signature_test_0 ... ok (gas usage est.: 389881208)
test alexandria_math::tests::ed25519_test::verify_signature_invalid ... ok (gas usage est.: 389229898)
test alexandria_math::tests::ed25519_test::verify_signature_test_3 ... ok (gas usage est.: 413750688)
test result: ok. 7 passed; 0 failed; 0 ignored; 143 filtered out;

After:

scarb test -f ed255
testing alexandria_math ...
running 7 tests
test alexandria_math::tests::ed25519_test::affine_point_op ... ok (gas usage est.: 411440)
test alexandria_math::tests::ed25519_test::verify_signature_invalid_length ... ok (gas usage est.: 5272652)
test alexandria_math::tests::ed25519_test::verify_signature_test_2 ... ok (gas usage est.: 390378618)
test alexandria_math::tests::ed25519_test::verify_signature_test_1 ... ok (gas usage est.: 382566258)
test alexandria_math::tests::ed25519_test::verify_signature_test_0 ... ok (gas usage est.: 389727588)
test alexandria_math::tests::ed25519_test::verify_signature_invalid ... ok (gas usage est.: 389076278)
test alexandria_math::tests::ed25519_test::verify_signature_test_3 ... ok (gas usage est.: 413597068)
test result: ok. 7 passed; 0 failed; 0 ignored; 143 filtered out;

@0xLucqs 0xLucqs merged commit da0cefa into keep-starknet-strange:main Aug 21, 2024
4 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Aug 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants