Implement the Sum trait for all AffineCurve (arkworks-rs#289)

* impl Sum for all AffineCurve

* update the CHANGELOG

* lint

* lint

* lint!

* lint again

* avoid linter battle; no more killing

Co-authored-by: Dev Ojha <[email protected]>

Author: weikengchen

.github/.markdownlint.yml

@@ -11,3 +11,4 @@ MD025: false
 MD033: false
 # MD036 no-emphasis-as-heading
 MD036: false
+MD041: false

.github/workflows/mdlinter.yml

@@ -28,3 +28,8 @@ jobs:
           MARKDOWN_CONFIG_FILE: .markdownlint.yml
           VALIDATE_PROTOBUF: false
           VALIDATE_JSCPD: false
+          # use Python Pylint as the only linter to avoid conflicts
+          VALIDATE_PYTHON_BLACK: false
+          VALIDATE_PYTHON_FLAKE8: false
+          VALIDATE_PYTHON_ISORT: false
+          VALIDATE_PYTHON_MYPY: false

.hooks/pre-commit

@@ -1,7 +1,6 @@
 #!/usr/bin/env bash
 
-rustfmt --version &>/dev/null
-if [ $? != 0 ]; then
+if rustfmt --version &>/dev/null; then
         printf "[pre_commit] \033[0;31merror\033[0m: \"rustfmt\" not available. \n"
         printf "[pre_commit] \033[0;31merror\033[0m: rustfmt can be installed via - \n"
         printf "[pre_commit] $ rustup component add rustfmt \n"
@@ -12,10 +11,9 @@ problem_files=()
 
 # collect ill-formatted files
 for file in $(git diff --name-only --cached); do
-        if [ ${file: -3} == ".rs" ]; then
-                rustfmt +stable --check $file &>/dev/null
-                if [ $? != 0 ]; then
-                        problem_files+=($file)
+        if [ "${file: -3}" == ".rs" ]; then
+                if rustfmt +stable --check "$file" &>/dev/null; then
+                        problem_files+=("$file")
                 fi
         fi
 done
@@ -26,10 +24,10 @@ if [ ${#problem_files[@]} == 0 ]; then
 else
   # reformat the files that need it and re-stage them.
   printf "[pre_commit] the following files were rustfmt'd before commit: \n"
-  for file in ${problem_files[@]}; do
-    rustfmt +stable $file
-    git add $file
-    printf "\033[0;32m    $file\033[0m \n"
+  for file in "${problem_files[@]}"; do
+    rustfmt +stable "$file"
+    git add "$file"
+    printf "\033[0;32m    %s\033[0m \n" "$file"
   done
 fi
 

CHANGELOG.md

@@ -13,14 +13,15 @@
 - [\#261](https://github.com/arkworks-rs/algebra/pull/261) (ark-ff) Add support for 448-bit integers and fields.
 - [\#263](https://github.com/arkworks-rs/algebra/pull/263) (ark-ff) Add `From<iXXX>` implementations to fields.
 - [\#265](https://github.com/arkworks-rs/algebra/pull/265) (ark-serialize) Add hashing as an extension trait of `CanonicalSerialize`.
-- [\#280](https://github.com/arkworks-rs/algebra/pull/280) (ark-ff) Add `Into<BigUint>` and `From<BigUint>` implementations to `BigInteger` and `PrimeField`.
 - [\#279](https://github.com/arkworks-rs/algebra/pull/279) (ark-ec) Parallelize miller loop operations for BLS12.
+- [\#280](https://github.com/arkworks-rs/algebra/pull/280) (ark-ff) Add `Into<BigUint>` and `From<BigUint>` implementations to `BigInteger` and `PrimeField`.
+- [\#289](https://github.com/arkworks-rs/algebra/pull/289) (ark-ec) Add `Sum` implementation for all `AffineCurve`.
 
 ### Improvements
+
 - [\#279](https://github.com/arkworks-rs/algebra/pull/279) (ark-ec) Parallelize miller loop operations for BLS12.
 
 ### Bug fixes
-- [\#252](https://github.com/arkworks-rs/algebra/pull/252) (ark-ff) Fix prime field sampling when `REPR_SHIFT_BITS` is 64.
 
 - [\#252](https://github.com/arkworks-rs/algebra/pull/252) (ark-ff) Fix prime field sampling when `REPR_SHIFT_BITS` is 64.
 - [\#284](https://github.com/arkworks-rs/algebra/pull/284) (ark-poly-benches) Fix the panic `subgroup_fft_in_place` benchmark for MNT6-753's Fr.

ec/src/lib.rs

@@ -239,6 +239,8 @@ pub trait AffineCurve:
     + Zero
     + Neg<Output = Self>
     + Zeroize
+    + core::iter::Sum<Self>
+    + for<'a> core::iter::Sum<&'a Self>
     + From<<Self as AffineCurve>::Projective>
 {
     const COFACTOR: &'static [u64];

ec/src/models/short_weierstrass_jacobian.rs

@@ -278,6 +278,22 @@ impl<P: Parameters> Default for GroupAffine<P> {
     }
 }
 
+impl<P: Parameters> core::iter::Sum<Self> for GroupAffine<P> {
+    fn sum<I: Iterator<Item = Self>>(iter: I) -> Self {
+        iter.map(|x| x.into_projective())
+            .sum::<GroupProjective<P>>()
+            .into()
+    }
+}
+
+impl<'a, P: Parameters> core::iter::Sum<&'a Self> for GroupAffine<P> {
+    fn sum<I: Iterator<Item = &'a Self>>(iter: I) -> Self {
+        iter.map(|x| x.into_projective())
+            .sum::<GroupProjective<P>>()
+            .into()
+    }
+}
+
 /// Jacobian coordinates for a point on an elliptic curve in short Weierstrass form,
 /// over the base field `P::BaseField`. This struct implements arithmetic
 /// via the Jacobian formulae

poly/src/polynomial/multivariate/mod.rs

@@ -53,9 +53,7 @@ pub trait Term:
 
 /// Stores a term (monomial) in a multivariate polynomial.
 /// Each element is of the form `(variable, power)`.  
-#[derive(
-    Clone, PartialOrd, PartialEq, Eq, Hash, Default, CanonicalSerialize, CanonicalDeserialize,
-)]
+#[derive(Clone, PartialEq, Eq, Hash, Default, CanonicalSerialize, CanonicalDeserialize)]
 pub struct SparseTerm(Vec<(usize, usize)>);
 
 impl SparseTerm {
@@ -141,26 +139,32 @@ impl Deref for SparseTerm {
     }
 }
 
-impl Ord for SparseTerm {
+impl PartialOrd for SparseTerm {
     /// Sort by total degree. If total degree is equal then ordering
     /// is given by exponent weight in lower-numbered variables
     /// ie. `x_1 > x_2`, `x_1^2 > x_1 * x_2`, etc.
-    fn cmp(&self, other: &Self) -> Ordering {
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
         if self.degree() != other.degree() {
-            self.degree().cmp(&other.degree())
+            Some(self.degree().cmp(&other.degree()))
         } else {
             // Iterate through all variables and return the corresponding ordering
             // if they differ in variable numbering or power
             for (cur, other) in self.iter().zip(other.iter()) {
                 if other.0 == cur.0 {
                     if cur.1 != other.1 {
-                        return (cur.1).cmp(&other.1);
+                        return Some((cur.1).cmp(&other.1));
                     }
                 } else {
-                    return (other.0).cmp(&cur.0);
+                    return Some((other.0).cmp(&cur.0));
                 }
             }
-            Ordering::Equal
+            Some(Ordering::Equal)
         }
     }
 }
+
+impl Ord for SparseTerm {
+    fn cmp(&self, other: &Self) -> Ordering {
+        self.partial_cmp(other).unwrap()
+    }
+}

scripts/linkify_changelog.py

@@ -1,7 +1,7 @@
-import re
-import sys
 import fileinput
 import os
+import re
+import sys
 
 # Set this to the name of the repo, if you don't want it to be read from the filesystem.
 # It assumes the changelog file is in the root of the repo.
@@ -11,7 +11,7 @@
 # with the valid mark down formatted link to it. e.g.
 # " [\#number](https://github.com/arkworks-rs/template/pull/<number>)
 # Note that if the number is for a an issue, github will auto-redirect you when you click the link.
-# It is safe to run the script multiple times in succession. 
+# It is safe to run the script multiple times in succession.
 #
 # Example usage $ python3 linkify_changelog.py ../CHANGELOG.md
 changelog_path = sys.argv[1]
@@ -21,6 +21,10 @@
     repo_name = components[-2]
 
 for line in fileinput.input(inplace=True):
-    line = re.sub(r"\- #([0-9]*)", r"- [\\#\1](https://github.com/arkworks-rs/" + repo_name + r"/pull/\1)", line.rstrip())
+    line = re.sub(
+        r"\- #([0-9]*)",
+        r"- [\#\1](https://github.com/arkworks-rs/" + repo_name + r"/pull/\1)",
+        line.rstrip(),
+    )
     # edits the current file
-    print(line)
+    print(line)

scripts/test_vectors.py

@@ -1,46 +1,51 @@
-
 def generate_from_bytes_mod_order_test_vector(modulus):
     def gen_vector(number):
         byte_arr = convert_int_to_byte_vec(number)
         # s = str(number % modulus)
         # return "(" + byte_arr + ", \"" + s + "\"),"
         return byte_arr + ","
+
     data = ["vec!["]
-    
-    small_values_to_test = [0, 1, 255, 256, 256*256 + 255]
+
+    small_values_to_test = [0, 1, 255, 256, 256 * 256 + 255]
     modulus_bits = int((len(bin(modulus)[2:]) + 7) / 8) * 8
     values_to_test = small_values_to_test + [
-        modulus >> 8, 
-        (modulus >> 8) + 1, 
+        modulus >> 8,
+        (modulus >> 8) + 1,
         modulus - 1,
         modulus,
         modulus + 1,
         modulus * 2,
         modulus * 256,
         17 + (1 << modulus_bits),
         19 + (1 << modulus_bits) + modulus,
-        81 + (1 << modulus_bits) * 256 + modulus]
-    
+        81 + (1 << modulus_bits) * 256 + modulus,
+    ]
+
     for i in values_to_test:
         data += ["// " + str(i)]
         data += [gen_vector(i)]
-    
+
     data += ["];"]
-    return '\n'.join(data)
+    return "\n".join(data)
+
 
 def convert_int_to_byte_vec(number):
     s = bin(number)[2:]
     num_bytes = int((len(s) + 7) / 8)
     s = s.zfill(num_bytes * 8)
-    
+
     byte_arr = []
     for i in range(num_bytes):
-        byte = s[i*8: (i+1)*8]
+        byte = s[i * 8 : (i + 1) * 8]
         i = int(byte, 2)
         byte_arr += [str(i) + "u8"]
-    
-    data = ', '.join(byte_arr)
+
+    data = ", ".join(byte_arr)
     return "vec![" + data + "]"
 
-bls12_fr_mod = 52435875175126190479447740508185965837690552500527637822603658699938581184513
+
+bls12_fr_mod = (
+    52435875175126190479447740508185965837690552500527637822603658699938581184513
+)
 print(generate_from_bytes_mod_order_test_vector(bls12_fr_mod))

test-templates/src/fields.rs

@@ -1,4 +1,5 @@
 #![allow(unused)]
+#![allow(clippy::eq_op)]
 use ark_ff::fields::{FftField, FftParameters, Field, LegendreSymbol, PrimeField, SquareRootField};
 use ark_serialize::{buffer_bit_byte_size, Flags, SWFlags};
 use ark_std::io::Cursor;
@@ -313,11 +314,9 @@ pub fn fft_field_test<F: FftField>() {
     if let Some(small_subgroup_base) = F::FftParams::SMALL_SUBGROUP_BASE {
         let small_subgroup_base_adicity = F::FftParams::SMALL_SUBGROUP_BASE_ADICITY.unwrap();
         let large_subgroup_root_of_unity = F::large_subgroup_root_of_unity().unwrap();
-        assert_eq!(
-            large_subgroup_root_of_unity.pow([(1 << F::FftParams::TWO_ADICITY)
-                * (small_subgroup_base as u64).pow(small_subgroup_base_adicity)]),
-            F::one()
-        );
+        let pow = (1 << F::FftParams::TWO_ADICITY)
+            * (small_subgroup_base as u64).pow(small_subgroup_base_adicity);
+        assert_eq!(large_subgroup_root_of_unity.pow([pow]), F::one());
 
         for i in 0..F::FftParams::TWO_ADICITY {
             for j in 0..small_subgroup_base_adicity {

test-templates/src/groups.rs

@@ -1,4 +1,5 @@
 #![allow(unused)]
+#![allow(clippy::eq_op)]
 use ark_ec::group::Group;
 use ark_ff::{One, UniformRand, Zero};