From da0cefaa2c3b6bbd940e408582761d6348192f2d Mon Sep 17 00:00:00 2001 From: Milan Cermak Date: Wed, 21 Aug 2024 10:50:58 +0200 Subject: [PATCH] refactor: rm math package dependencies (#325) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Pull Request type Please check the type of change your PR introduces: - [ ] Bugfix - [ ] Feature - [ ] Code style update (formatting, renaming) - [x] 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 - [x] No ## Other information There's no regression in performance of the ed25519. Before this change: ```shell $ 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: ```shell 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; ``` --- Scarb.lock | 5 +--- packages/math/Scarb.toml | 8 ++---- packages/math/src/ed25519.cairo | 50 +++++++++++++++++++++++++-------- 3 files changed, 42 insertions(+), 21 deletions(-) diff --git a/Scarb.lock b/Scarb.lock index 46dadaab..eacbd883 100644 --- a/Scarb.lock +++ b/Scarb.lock @@ -41,10 +41,7 @@ dependencies = [ [[package]] name = "alexandria_math" -version = "0.2.0" -dependencies = [ - "alexandria_data_structures", -] +version = "0.2.1" [[package]] name = "alexandria_merkle_tree" diff --git a/packages/math/Scarb.toml b/packages/math/Scarb.toml index 120236be..b05035f6 100644 --- a/packages/math/Scarb.toml +++ b/packages/math/Scarb.toml @@ -1,6 +1,6 @@ [package] name = "alexandria_math" -version = "0.2.0" +version = "0.2.1" description = "A set of math libraries and algorithms" homepage = "https://github.com/keep-starknet-strange/alexandria/tree/main/packages/math" edition = "2023_11" @@ -8,9 +8,5 @@ edition = "2023_11" [tool] fmt.workspace = true -[dependencies] -# dependency due to ArrayTraitExt::concat in ed25519.cairo -alexandria_data_structures = { path = "../data_structures" } - [dev-dependencies] -cairo_test.workspace = true \ No newline at end of file +cairo_test.workspace = true diff --git a/packages/math/src/ed25519.cairo b/packages/math/src/ed25519.cairo index 5ba9dc6c..185e31b2 100644 --- a/packages/math/src/ed25519.cairo +++ b/packages/math/src/ed25519.cairo @@ -1,14 +1,11 @@ -use alexandria_data_structures::span_ext::SpanTraitExt; use alexandria_math::mod_arithmetics::{mult_mod, sqr_mod, div_mod, pow_mod, equality_mod}; use alexandria_math::sha512::{sha512, SHA512_LEN}; use alexandria_math::u512_arithmetics::{u512_add, u512_sub}; -use core::array::ArrayTrait; use core::integer::{u512, u512_safe_div_rem_by_u256}; use core::math::u256_inv_mod; use core::num::traits::{OverflowingMul, WideMul}; use core::option::OptionTrait; -use core::traits::Div; -use core::traits::TryInto; +use core::traits::{Div, TryInto}; // Subtraction without modulo operation // assumes a, b < modulo @@ -239,12 +236,12 @@ impl U256TryIntoPoint of TryInto { fn try_into(self: u256) -> Option { let mut x = 0; let mut y_span: Span = self.into(); - let mut y_le_span: Span = y_span.reversed().span(); + let mut y_le_span: Span = reverse(y_span); let last_byte = *y_le_span[31]; let _ = y_le_span.pop_back(); - let mut normed_array: Array = y_le_span.dedup(); + let mut normed_array: Array = dedup(y_le_span); normed_array.append(last_byte & ~0x80); let x_0: u256 = (last_byte.into() / 128) & 1; // bitshift of 255 @@ -361,8 +358,8 @@ pub fn verify_signature(msg: Span, signature: Span, pub_key: u256) -> let s: u256 = *signature[1]; let s_span: Span = s.into(); - let reversed_s_span = s_span.reversed(); - let s: u256 = reversed_s_span.span().into(); + let reversed_s_span = reverse(s_span); + let s: u256 = reversed_s_span.into(); if (s >= l) { return false; } @@ -376,11 +373,15 @@ pub fn verify_signature(msg: Span, signature: Span, pub_key: u256) -> let A_prime: Point = A_prime_opt.unwrap(); let r_bytes: Span = r.into(); - let r_bytes = r_bytes.reversed().span(); + let r_bytes = reverse(r_bytes); let pub_key_bytes: Span = pub_key.into(); - let pub_key_bytes = pub_key_bytes.reversed().span(); + let pub_key_bytes = reverse(pub_key_bytes); + + let mut hashable = array![]; + hashable.append_span(r_bytes); + hashable.append_span(pub_key_bytes); + hashable.append_span(msg); - let hashable = r_bytes.concat(pub_key_bytes).span().concat(msg); // k = SHA512(dom2(F, C) -> empty string || R -> half of sig || A -> pub_key || PH(M) -> // identity function for msg) let k: Array = sha512(hashable); @@ -391,3 +392,30 @@ pub fn verify_signature(msg: Span, signature: Span, pub_key: u256) -> check_group_equation(s, R, k_reduced, A_prime) } + +// reverse and dedup are helper functions copy-pasted here from +// the data_structures SpanTraitExt implementation to prevent +// dependency of the math package on the data_structures package +// and all its heavy dependencies + +fn reverse(mut span: Span) -> Span { + let mut res = array![]; + while let Option::Some(v) = span.pop_back() { + res.append(v.clone()); + }; + res.span() +} + +fn dedup(mut span: Span) -> Array { + let mut last_value = span.pop_front().unwrap(); + let mut ret = array![last_value.clone()]; + + for v in span { + if (last_value != v) { + last_value = v; + ret.append(v.clone()); + } + }; + + ret +}