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

Fibonacci Stark Prover #59

Merged
merged 89 commits into from
Mar 2, 2023
Merged

Fibonacci Stark Prover #59

merged 89 commits into from
Mar 2, 2023

Conversation

pablodeymo
Copy link
Contributor

@pablodeymo pablodeymo commented Feb 13, 2023

Basic implementation of the stark proving system for the fibonacci sequence. There are a lot of TODOs left, which we'll tackle in other PRs, like adding fiat-shamir, zerofiers, etc.

A few things were added out of need:

  • A representative() method for FieldElements, which returns the field element's value in plain form (if the felt is in montgomery form, this returns the actual value the user expects, not its montgomery representation).
  • A compose function to compute the composition of two polynomials.

@pablodeymo pablodeymo changed the title [WIP] Fri operation [WIP] FRI operation Feb 13, 2023
prover/src/fri/mod.rs Outdated Show resolved Hide resolved
prover/src/fri/mod.rs Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Feb 16, 2023

Codecov Report

Merging #59 (d2ad4a4) into main (294ee35) will decrease coverage by 0.16%.
The diff coverage is 93.51%.

@@            Coverage Diff             @@
##             main      #59      +/-   ##
==========================================
- Coverage   95.79%   95.63%   -0.16%     
==========================================
  Files          45       49       +4     
  Lines        4680     5155     +475     
==========================================
+ Hits         4483     4930     +447     
- Misses        197      225      +28     
Impacted Files Coverage Δ
crypto/src/merkle_tree/mod.rs 100.00% <ø> (ø)
math/src/field/extensions/cubic.rs 86.33% <0.00%> (-1.64%) ⬇️
math/src/field/extensions/quadratic.rs 96.42% <0.00%> (-2.12%) ⬇️
math/src/field/fields/u64_prime_field.rs 97.83% <0.00%> (-1.62%) ⬇️
math/src/field/test_fields/u64_test_field.rs 65.78% <0.00%> (-5.64%) ⬇️
math/src/field/traits.rs 87.50% <ø> (ø)
math/src/unsigned_integer/element.rs 98.02% <ø> (ø)
math/src/polynomial.rs 95.57% <76.19%> (-2.24%) ⬇️
crypto/src/merkle_tree/proof.rs 99.04% <90.90%> (-0.96%) ⬇️
proving-system/stark/src/lib.rs 97.03% <97.03%> (ø)
... and 7 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@MauroToscano MauroToscano changed the title [WIP] FRI operation Fibonacci Stark Prover Mar 1, 2023
@jrchatruc jrchatruc marked this pull request as ready for review March 1, 2023 20:15
@jrchatruc jrchatruc requested review from a team, schouhy and ajgara as code owners March 1, 2023 20:15
@@ -151,6 +151,11 @@ where
fn from_base_type(x: Self::BaseType) -> Self::BaseType {
MontgomeryAlgorithms::cios(&x, &C::R2, &C::MODULUS, &C::MU)
}

// TO DO: Add tests for representatives
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make an issue to track this TODO

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could label it as "good first issue".

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, created it here #138

@@ -127,6 +127,10 @@ where
fn from_base_type(x: [FieldElement<Q::BaseField>; 3]) -> [FieldElement<Q::BaseField>; 3] {
x
}

fn representative(_x: Self::BaseType) -> Self::BaseType {
todo!()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for this

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -105,6 +105,10 @@ where
fn from_base_type(x: [FieldElement<Q::BaseField>; 2]) -> [FieldElement<Q::BaseField>; 2] {
x
}

fn representative(_x: Self::BaseType) -> Self::BaseType {
todo!()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for this

Copy link
Collaborator

@ilitteri ilitteri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ilitteri ilitteri enabled auto-merge March 2, 2023 13:53
@ilitteri ilitteri added this pull request to the merge queue Mar 2, 2023
@entropidelic
Copy link
Contributor

LGTM!

Merged via the queue into main with commit 3c681a3 Mar 2, 2023
@ilitteri ilitteri deleted the fri-operation branch March 2, 2023 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants