Cleanup/moving primitive roots to consts (#38)

* adding bare changes for batch verification

* adding some comments

* adding more comments

* moving back to sha2

* removing a test which is no longer needed. Removing methods no longer needed

* updates to method visibility, updating tests

* fmt fixes

* clean up

* cleanup, optimization, inline docs

* removing unwanted const

* more docs and cleanup

* formatting

* removing unwanted comments

* cargo fmt and clippy

* adding test for point at infinity

* cleaner errors, cleanup

* adding another test case

* removing unwanted errors

* adding fixes per comments

* adding 4844 spec references

* comment fixes

* formatting, adding index out of bound check, removing print statement

* removing unwanted test, adding test for evaluate_polynomial_in_evaluation_form

* moving test to bottom section

* Update src/polynomial.rs

Co-authored-by: Samuel Laferriere <samlaf92@gmail.com>

* Update src/kzg.rs

Co-authored-by: Samuel Laferriere <samlaf92@gmail.com>

* Update src/kzg.rs

Co-authored-by: Samuel Laferriere <samlaf92@gmail.com>

* Update src/kzg.rs

Co-authored-by: Samuel Laferriere <samlaf92@gmail.com>

* Update src/helpers.rs

Co-authored-by: Samuel Laferriere <samlaf92@gmail.com>

* updating deps, and toolchain to 1.84

* removing errors test, no longer useful

* adding to_byte_array arg explanation

* fmt fixes

* fmt and clippy fixes

* fixing function names and fmt

* clippy fixes

* moving primitive root to consts, adding helper function and changing call methods.

* removing function

* wrong branch updated

* removing is_zero

* comment updates

* using macro for roots consts

* adding explanation to PRIMITIVE_ROOTS_OF_UNITY

* Update src/kzg.rs

Co-authored-by: Samuel Laferriere <samlaf92@gmail.com>

* Update src/helpers.rs

Co-authored-by: Samuel Laferriere <samlaf92@gmail.com>

* comment change, renames and doc fixes

* cargo fmt

---------

Co-authored-by: anupsv <6407789+anupsv@users.noreply.github.com>
Co-authored-by: Samuel Laferriere

src/consts.rs

@@ -1,4 +1,4 @@
-use ark_bn254::{Fq2, G2Affine};
+use ark_bn254::{Fq2, Fr, G2Affine};
 use ark_ff::MontFp;
 
 pub const BYTES_PER_FIELD_ELEMENT: usize = 32;
@@ -10,6 +10,47 @@ pub const FIAT_SHAMIR_PROTOCOL_DOMAIN: &[u8] = b"EIGENDA_FSBLOBVERIFY_V1_"; // A
 /// Ref: https://github.com/ethereum/consensus-specs/blob/master/specs/deneb/polynomial-commitments.md#blob
 pub const RANDOM_CHALLENGE_KZG_BATCH_DOMAIN: &[u8] = b"EIGENDA_RCKZGBATCH___V1_"; // Adapted from 4844
 
+/// These are the primitive 2^nth roots of unity. `2^28`
+/// is the largest power of two that divides `r - 1`, therefore there are no primitive
+/// roots of unity for higher powers of 2 in Fr.). Given a blob
+/// length, we calculate the number of field elements needed and then get the
+/// value of the actual power of 2 which is less than or equal to 28. We then use
+/// this value to get the primitive root of unity from the array below to expand
+/// to the roots of unity needed. The number of expanded roots elements will be the
+/// same as the power of 2 i.e if `8th` power of 2 was calculated then we get `2^8 = 256`
+/// expanded roots of unity.   
+pub const PRIMITIVE_ROOTS_OF_UNITY: [Fr; 29] = [
+    MontFp!("1"),
+    MontFp!("21888242871839275222246405745257275088548364400416034343698204186575808495616"),
+    MontFp!("21888242871839275217838484774961031246007050428528088939761107053157389710902"),
+    MontFp!("19540430494807482326159819597004422086093766032135589407132600596362845576832"),
+    MontFp!("14940766826517323942636479241147756311199852622225275649687664389641784935947"),
+    MontFp!("4419234939496763621076330863786513495701855246241724391626358375488475697872"),
+    MontFp!("9088801421649573101014283686030284801466796108869023335878462724291607593530"),
+    MontFp!("10359452186428527605436343203440067497552205259388878191021578220384701716497"),
+    MontFp!("3478517300119284901893091970156912948790432420133812234316178878452092729974"),
+    MontFp!("6837567842312086091520287814181175430087169027974246751610506942214842701774"),
+    MontFp!("3161067157621608152362653341354432744960400845131437947728257924963983317266"),
+    MontFp!("1120550406532664055539694724667294622065367841900378087843176726913374367458"),
+    MontFp!("4158865282786404163413953114870269622875596290766033564087307867933865333818"),
+    MontFp!("197302210312744933010843010704445784068657690384188106020011018676818793232"),
+    MontFp!("20619701001583904760601357484951574588621083236087856586626117568842480512645"),
+    MontFp!("20402931748843538985151001264530049874871572933694634836567070693966133783803"),
+    MontFp!("421743594562400382753388642386256516545992082196004333756405989743524594615"),
+    MontFp!("12650941915662020058015862023665998998969191525479888727406889100124684769509"),
+    MontFp!("11699596668367776675346610687704220591435078791727316319397053191800576917728"),
+    MontFp!("15549849457946371566896172786938980432421851627449396898353380550861104573629"),
+    MontFp!("17220337697351015657950521176323262483320249231368149235373741788599650842711"),
+    MontFp!("13536764371732269273912573961853310557438878140379554347802702086337840854307"),
+    MontFp!("12143866164239048021030917283424216263377309185099704096317235600302831912062"),
+    MontFp!("934650972362265999028062457054462628285482693704334323590406443310927365533"),
+    MontFp!("5709868443893258075976348696661355716898495876243883251619397131511003808859"),
+    MontFp!("19200870435978225707111062059747084165650991997241425080699860725083300967194"),
+    MontFp!("7419588552507395652481651088034484897579724952953562618697845598160172257810"),
+    MontFp!("2082940218526944230311718225077035922214683169814847712455127909555749686340"),
+    MontFp!("19103219067921713944291392827692070036145651957329286315305642004821462161904"),
+];
+
 pub const G2_TAU_FOR_TEST_SRS_3000: G2Affine = G2Affine::new_unchecked(
     Fq2::new(
         MontFp!("7912312892787135728292535536655271843828059318189722219035249994421084560563"),

src/helpers.rs

@@ -7,7 +7,7 @@ use std::cmp;
 
 use crate::{
     arith,
-    consts::{BYTES_PER_FIELD_ELEMENT, SIZE_OF_G1_AFFINE_COMPRESSED},
+    consts::{BYTES_PER_FIELD_ELEMENT, PRIMITIVE_ROOTS_OF_UNITY, SIZE_OF_G1_AFFINE_COMPRESSED},
     errors::KzgError,
     traits::ReadPointFromBytes,
 };
@@ -435,3 +435,36 @@ pub fn g1_lincomb(points: &[G1Affine], scalars: &[Fr]) -> Result<G1Affine, KzgEr
     // This is typically needed as most protocols expect points in affine form
     Ok(lincomb.into_affine())
 }
+
+/// Retrieves and converts a primitive root of unity to a field element
+///
+/// # Arguments
+/// * `index` - Index of the primitive root to retrieve from PRIMITIVE_ROOTS_OF_UNITY array
+///
+/// # Returns
+/// * `Result<Fr, KzgError>` - Field element representation of the primitive root if successful,
+///                           or KzgError if index is invalid or conversion fails
+///
+/// # Errors
+/// - Returns KzgError::GenericError if:
+///   - Index is out of bounds for PRIMITIVE_ROOTS_OF_UNITY array
+///   - BigInt conversion to field element fails
+///
+/// # Details
+/// - Looks up a primitive root of unity from a predefined array using the given index
+/// - Converts the BigInt representation to an Fr field element
+/// - Commonly used in FFT and polynomial operations requiring roots of unity
+///
+/// # Example
+/// ```
+/// use rust_kzg_bn254::helpers::get_primitive_root_of_unity;
+/// let root = get_primitive_root_of_unity(0); // Gets first primitive root
+/// ```
+/// Gets the primitive root of unity of order 2^power.
+/// For example, power=3 returns a primitive 8th root of unity.
+pub fn get_primitive_root_of_unity(power: usize) -> Result<Fr, KzgError> {
+    PRIMITIVE_ROOTS_OF_UNITY
+        .get(power)
+        .ok_or_else(|| KzgError::GenericError("power must be <= 28".to_string()))
+        .copied()
+}

src/kzg.rs

@@ -16,7 +16,7 @@ use ark_ec::{pairing::Pairing, AffineRepr, CurveGroup, VariableBaseMSM};
 use ark_ff::{BigInteger, Field, PrimeField};
 use ark_poly::{EvaluationDomain, GeneralEvaluationDomain};
 use ark_serialize::{CanonicalSerialize, Read};
-use ark_std::{iterable::Iterable, ops::Div, str::FromStr, One, Zero};
+use ark_std::{iterable::Iterable, ops::Div, One, Zero};
 use crossbeam_channel::{bounded, Sender};
 use num_traits::ToPrimitive;
 use rayon::iter::{IntoParallelRefIterator, ParallelIterator};
@@ -114,16 +114,11 @@ impl KZG {
             ));
         }
 
-        // Get the primitive roots of unity
-        let primitive_roots_of_unity = Self::get_primitive_roots_of_unity()?;
-
         // Find the root of unity corresponding to the calculated log2 value
-        let found_root_of_unity = primitive_roots_of_unity
-            .get(log2_of_evals as usize)
-            .ok_or_else(|| KzgError::GenericError("Root of unity not found".to_string()))?;
+        let root_of_unity = helpers::get_primitive_root_of_unity(log2_of_evals.into())?;
 
         // Expand the root to get all the roots of unity
-        let mut expanded_roots_of_unity = Self::expand_root_of_unity(found_root_of_unity);
+        let mut expanded_roots_of_unity = Self::expand_root_of_unity(&root_of_unity);
 
         // Remove the last element to avoid duplication
         expanded_roots_of_unity.truncate(expanded_roots_of_unity.len() - 1);
@@ -194,48 +189,6 @@ impl KZG {
         roots
     }
 
-    /// Precompute the primitive roots of unity for binary powers that divide r - 1
-    /// TODO(anupsv): Move this to the constants file. Ref: https://github.com/Layr-Labs/rust-kzg-bn254/issues/31
-    fn get_primitive_roots_of_unity() -> Result<Vec<Fr>, KzgError> {
-        let data: [&str; 29] = [
-            "1",
-            "21888242871839275222246405745257275088548364400416034343698204186575808495616",
-            "21888242871839275217838484774961031246007050428528088939761107053157389710902",
-            "19540430494807482326159819597004422086093766032135589407132600596362845576832",
-            "14940766826517323942636479241147756311199852622225275649687664389641784935947",
-            "4419234939496763621076330863786513495701855246241724391626358375488475697872",
-            "9088801421649573101014283686030284801466796108869023335878462724291607593530",
-            "10359452186428527605436343203440067497552205259388878191021578220384701716497",
-            "3478517300119284901893091970156912948790432420133812234316178878452092729974",
-            "6837567842312086091520287814181175430087169027974246751610506942214842701774",
-            "3161067157621608152362653341354432744960400845131437947728257924963983317266",
-            "1120550406532664055539694724667294622065367841900378087843176726913374367458",
-            "4158865282786404163413953114870269622875596290766033564087307867933865333818",
-            "197302210312744933010843010704445784068657690384188106020011018676818793232",
-            "20619701001583904760601357484951574588621083236087856586626117568842480512645",
-            "20402931748843538985151001264530049874871572933694634836567070693966133783803",
-            "421743594562400382753388642386256516545992082196004333756405989743524594615",
-            "12650941915662020058015862023665998998969191525479888727406889100124684769509",
-            "11699596668367776675346610687704220591435078791727316319397053191800576917728",
-            "15549849457946371566896172786938980432421851627449396898353380550861104573629",
-            "17220337697351015657950521176323262483320249231368149235373741788599650842711",
-            "13536764371732269273912573961853310557438878140379554347802702086337840854307",
-            "12143866164239048021030917283424216263377309185099704096317235600302831912062",
-            "934650972362265999028062457054462628285482693704334323590406443310927365533",
-            "5709868443893258075976348696661355716898495876243883251619397131511003808859",
-            "19200870435978225707111062059747084165650991997241425080699860725083300967194",
-            "7419588552507395652481651088034484897579724952953562618697845598160172257810",
-            "2082940218526944230311718225077035922214683169814847712455127909555749686340",
-            "19103219067921713944291392827692070036145651957329286315305642004821462161904",
-        ];
-        data.iter()
-            .map(Fr::from_str)
-            .collect::<Result<Vec<_>, _>>()
-            .map_err(|_| {
-                KzgError::GenericError("Failed to parse primitive roots of unity".to_string())
-            })
-    }
-
     /// helper function to get g1 points
     pub fn get_g1_points(&self) -> Vec<G1Affine> {
         self.g1.to_vec()

tests/kzg_test.rs

@@ -6,7 +6,8 @@ mod tests {
     use lazy_static::lazy_static;
     use rand::Rng;
     use rust_kzg_bn254::{
-        blob::Blob, errors::KzgError, helpers, kzg::KZG, polynomial::PolynomialCoeffForm,
+        blob::Blob, consts::PRIMITIVE_ROOTS_OF_UNITY, errors::KzgError, helpers, kzg::KZG,
+        polynomial::PolynomialCoeffForm,
     };
     use std::{env, fs::File, io::BufReader};
     const GETTYSBURG_ADDRESS_BYTES: &[u8] = "Fourscore and seven years ago our fathers brought forth, on this continent, a new nation, conceived in liberty, and dedicated to the proposition that all men are created equal. Now we are engaged in a great civil war, testing whether that nation, or any nation so conceived, and so dedicated, can long endure. We are met on a great battle-field of that war. We have come to dedicate a portion of that field, as a final resting-place for those who here gave their lives, that that nation might live. It is altogether fitting and proper that we should do this. But, in a larger sense, we cannot dedicate, we cannot consecrate—we cannot hallow—this ground. The brave men, living and dead, who struggled here, have consecrated it far above our poor power to add or detract. The world will little note, nor long remember what we say here, but it can never forget what they did here. It is for us the living, rather, to be dedicated here to the unfinished work which they who fought here have thus far so nobly advanced. It is rather for us to be here dedicated to the great task remaining before us—that from these honored dead we take increased devotion to that cause for which they here gave the last full measure of devotion—that we here highly resolve that these dead shall not have died in vain—that this nation, under God, shall have a new birth of freedom, and that government of the people, by the people, for the people, shall not perish from the earth.".as_bytes();
@@ -463,6 +464,50 @@ mod tests {
         }
     }
 
+    #[test]
+    fn test_primitive_roots_from_bigint_to_fr() {
+        let data: [&str; 29] = [
+            "1",
+            "21888242871839275222246405745257275088548364400416034343698204186575808495616",
+            "21888242871839275217838484774961031246007050428528088939761107053157389710902",
+            "19540430494807482326159819597004422086093766032135589407132600596362845576832",
+            "14940766826517323942636479241147756311199852622225275649687664389641784935947",
+            "4419234939496763621076330863786513495701855246241724391626358375488475697872",
+            "9088801421649573101014283686030284801466796108869023335878462724291607593530",
+            "10359452186428527605436343203440067497552205259388878191021578220384701716497",
+            "3478517300119284901893091970156912948790432420133812234316178878452092729974",
+            "6837567842312086091520287814181175430087169027974246751610506942214842701774",
+            "3161067157621608152362653341354432744960400845131437947728257924963983317266",
+            "1120550406532664055539694724667294622065367841900378087843176726913374367458",
+            "4158865282786404163413953114870269622875596290766033564087307867933865333818",
+            "197302210312744933010843010704445784068657690384188106020011018676818793232",
+            "20619701001583904760601357484951574588621083236087856586626117568842480512645",
+            "20402931748843538985151001264530049874871572933694634836567070693966133783803",
+            "421743594562400382753388642386256516545992082196004333756405989743524594615",
+            "12650941915662020058015862023665998998969191525479888727406889100124684769509",
+            "11699596668367776675346610687704220591435078791727316319397053191800576917728",
+            "15549849457946371566896172786938980432421851627449396898353380550861104573629",
+            "17220337697351015657950521176323262483320249231368149235373741788599650842711",
+            "13536764371732269273912573961853310557438878140379554347802702086337840854307",
+            "12143866164239048021030917283424216263377309185099704096317235600302831912062",
+            "934650972362265999028062457054462628285482693704334323590406443310927365533",
+            "5709868443893258075976348696661355716898495876243883251619397131511003808859",
+            "19200870435978225707111062059747084165650991997241425080699860725083300967194",
+            "7419588552507395652481651088034484897579724952953562618697845598160172257810",
+            "2082940218526944230311718225077035922214683169814847712455127909555749686340",
+            "19103219067921713944291392827692070036145651957329286315305642004821462161904",
+        ];
+        let fr_s = data
+            .iter()
+            .map(|s: &&str| Fr::from_str(*s).unwrap())
+            .collect::<Vec<_>>();
+
+        for i in 0..PRIMITIVE_ROOTS_OF_UNITY.len() {
+            let root_of_unity_at_index = PRIMITIVE_ROOTS_OF_UNITY[i];
+            assert_eq!(root_of_unity_at_index, fr_s[i]);
+        }
+    }
+
     #[test]
     fn test_g2_tau_in_group() {
         let kzg = &KZG_INSTANCE;