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

Arrabbiata: move selectors into setup #3073

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

dannywillems
Copy link
Member

No description provided.

Moving it into the interpreter.
The idea is to delegate the whole control flow and the construction of the
constraints and environment into the interpreter file.
The size of the application is computated as the size of the SRS minus the size
of the verifier circuit.
This will be used to build the selectors at setup time.
The circuit gates field contains the description of the program being executed.
The circuti gates contains the semantic of the actual computation, which can be
seen as the type of the program being executed. The "type of the program" is
described by a list of "gadgets".
To include the circuit gates and the app size
The value was pretty old. The value will be updated over time, while we are
augmenting the verifier circuit
The gadget activation will be contained in the setup.
The circuit shape is now defined in the setup phase.
@dannywillems dannywillems force-pushed the dw/use-selectors-in-setup branch from b4b4524 to 47d62f5 Compare February 26, 2025 22:50
@dannywillems dannywillems force-pushed the dw/use-selectors-in-setup branch from e435e95 to 48a1902 Compare February 27, 2025 20:55
Comment on lines -1052 to -1103
///
/// For a step i + 1, the verifier circuit receives as public input the
/// following values:
///
/// - The commitments to the previous witnesses.
/// - The previous challenges (α_{i}, β_{i}, γ_{i}) - the challenges β and γ
/// are used by the permutation argument where α is used by the quotient
/// polynomial, generated after also absorbing the accumulator of the
/// permutation argument.
/// - The previous accumulators (acc_1, ..., acc_17).
/// - The previous output z_i.
/// - The initial input z_0.
/// - The natural i describing the previous step.
///
/// The control flow is as follow:
/// - We compute the hash of the previous commitments and verify the hash
/// corresponds to the public input:
///
/// ```text
/// hash = H(i, acc_1, ..., acc_17, z_0, z_i)
/// ```
///
/// - We also have to check that the previous challenges (α, β, γ) have been
/// correctly generated. Therefore, we must compute the hashes of the
/// witnesses and verify they correspond to the public input.
///
/// TODO
///
/// - We compute the output of the application (TODO)
///
/// ```text
/// z_(i + 1) = F(w_i, z_i)
/// ```
///
/// - We compute the MSM (verifier)
///
/// ```text
/// acc_(i + 1)_j = acc_i + r C_j
/// ```
/// And also the cross-terms:
///
/// ```text
/// E = E1 - r T1 - r^2 T2 - ... - r^d T^d + r^(d+1) E2
/// = E1 - r (T1 + r (T2 + ... + r T^(d - 1)) - r E2)
/// ```
/// where (d + 1) is the degree of the highest gate.
///
/// - We compute the next hash we give to the next instance
///
/// ```text
/// hash' = H(i + 1, acc'_1, ..., acc'_17, z_0, z_(i + 1))
/// ```
Copy link
Contributor

Choose a reason for hiding this comment

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

is the suppression of the comment voluntary ?

@@ -199,6 +207,27 @@ where
// suppose we have the same circuit on both curves for now.
let app_size = srs_size - VERIFIER_CIRCUIT_SIZE;

// Build the selectors for both circuits.
// FIXME: we suppose we have the same circuit on both curve for now.
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand the app only handle one gadget.
Note that a generalisation will be needed at some point

@@ -23,8 +23,7 @@ pub mod witness;
pub const MAX_DEGREE: usize = 5;

/// The minimum SRS size required to use Nova, in base 2.
/// Requiring at least 2^16 to perform 16bits range checks.
pub const MIN_SRS_LOG2_SIZE: usize = 16;
pub const MIN_SRS_LOG2_SIZE: usize = 8;
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this be derived from the verification circuit size ?

/// The gadget defining the app.
///
/// For now, the application is considered to be a one-line computation.
/// However, we want to see the application as a collection of reusable
Copy link
Contributor

Choose a reason for hiding this comment

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

ah ok this addresses my previous comment on commit 25febf9

@@ -57,8 +57,8 @@ pub const MAXIMUM_FIELD_SIZE_IN_BITS: u64 = 255;
/// verifier circuit.
pub const NUMBER_OF_VALUES_TO_ABSORB_PUBLIC_IO: usize = NUMBER_OF_COLUMNS * 2;

/// The number of selectors used in the circuit.
pub const NUMBER_OF_SELECTORS: usize =
/// The number of gadgets supported by the program
Copy link
Contributor

Choose a reason for hiding this comment

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

isn't this the max nb of gadgets ?

[PolyComm<E1>; NUMBER_OF_GADGETS],
[PolyComm<E2>; NUMBER_OF_GADGETS],
) = {
// We initialize to the blinder to avoid the identity element.
Copy link
Contributor

Choose a reason for hiding this comment

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

why avoid the identity
Also we shouldn't blind the commitment, which are public

@@ -287,7 +282,7 @@ fn test_regression_witness_structure_sizeof() {
// thining about the memory efficiency of the codebase.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// thining about the memory efficiency of the codebase.
// thinking about the memory efficiency of the codebase.

@marcbeunardeau88
Copy link
Contributor

Note that the last commit is not verified

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.

2 participants