Skip to content
This repository has been archived by the owner on Oct 1, 2024. It is now read-only.

Uni-Stark Challenges #2

Merged
merged 49 commits into from
Sep 23, 2024
Merged

Uni-Stark Challenges #2

merged 49 commits into from
Sep 23, 2024

Conversation

topanisto
Copy link
Collaborator

Support for challenges: main additions will include

  • Splitting prove() into run_stage() and finish() to handle higher stage trace generation.
  • New trait NextStageTraceCallback and CommittedData to handle multiple traces.
  • Adjustments to Folder, check_constraints, and SymbolicAirBuilder for compatibility with assertions over traces of different stages.

@topanisto topanisto changed the base branch from main to uni-stark-with-fixed August 8, 2024 13:03
uni-stark/src/check_constraints.rs Outdated Show resolved Hide resolved
uni-stark/src/prover.rs Outdated Show resolved Hide resolved
uni-stark/src/prover.rs Outdated Show resolved Hide resolved
uni-stark/src/prover.rs Outdated Show resolved Hide resolved
@Schaeff Schaeff marked this pull request as ready for review September 12, 2024 19:47
Copy link

@georgwiese georgwiese left a comment

Choose a reason for hiding this comment

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

Cool!

I still have to look through the proving code in more detail, but have some comments on the rest of the changes already.

keccak-air/src/air.rs Show resolved Hide resolved
}

#[derive(Debug, Serialize, Deserialize)]
pub struct Commitments<Com> {
pub(crate) trace: Com,
pub(crate) stages: Vec<Com>,

Choose a reason for hiding this comment

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

Suggested change
pub(crate) stages: Vec<Com>,
pub(crate) traces_by_stage: Vec<Com>,

Comment on lines 42 to 43
pub(crate) stages_local: Vec<Vec<Challenge>>,
pub(crate) stages_next: Vec<Vec<Challenge>>,

Choose a reason for hiding this comment

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

Hmm, again, I would still include "trace" in the name. traces_by_stage_local?

Choose a reason for hiding this comment

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

I guess that's more a general comment: What's a stage? To me it's just a number, for anything else I think it would be better to give a more specific name.

uni-stark/src/proof.rs Show resolved Hide resolved
uni-stark/src/verifier.rs Show resolved Hide resolved
uni-stark/src/verifier.rs Outdated Show resolved Hide resolved
uni-stark/src/verifier.rs Outdated Show resolved Hide resolved
uni-stark/src/verifier.rs Outdated Show resolved Hide resolved
uni-stark/src/prover.rs Outdated Show resolved Hide resolved
Base automatically changed from uni-stark-with-fixed to main September 17, 2024 08:55
Copy link

@georgwiese georgwiese left a comment

Choose a reason for hiding this comment

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

LGTM, but clippy is failing.

@Schaeff Schaeff merged commit b38d3fa into main Sep 23, 2024
2 of 3 checks passed
@Schaeff Schaeff deleted the uni-stark-challenges-2 branch September 23, 2024 14:19
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.

3 participants