Verifier core audit comments #1360

Gali-StarkWare · 2025-09-11T08:54:04Z

Verifier core audit comments

Audit comments

This change is

Gali-StarkWare · 2025-09-11T08:54:20Z

Verifier core audit comments #1360 👈 (View in Graphite)
sharp7

This stack of pull requests is managed by Graphite. Learn more about stacking.

Gali-StarkWare

@Gali-StarkWare reviewed all commit messages.
Reviewable status: 0 of 23 files reviewed, 1 unresolved discussion

stwo_cairo_verifier/crates/cairo_air/src/cairo_air.cairo line 520 at r1 (raw file):

pub impl CairoAirImpl of Air<CairoAir> {
    fn composition_log_degree_bound(self: @CairoAir) -> u32 {
        // TODO(audit): Change name of field to composition_log_degree_bound. Done

Done

Gali-StarkWare

Reviewable status: 0 of 23 files reviewed, all discussions resolved

stwo_cairo_verifier/crates/cairo_air/src/cairo_air.cairo line 520 at r1 (raw file):

Previously, Gali-StarkWare wrote…

Done

Done in #1351

Gali-StarkWare

Reviewable status: 0 of 23 files reviewed, 1 unresolved discussion

stwo_cairo_verifier/crates/cairo_air/src/lib.cairo line 188 at r1 (raw file):

    let CairoProof { claim, interaction_pow, interaction_claim, stark_proof, channel_salt } = proof;

    // TODO(audit): assert that len of commitments is 4. Done

Done in #1318

Gali-StarkWare

Reviewable status: 0 of 23 files reviewed, 3 unresolved discussions

stwo_cairo_verifier/crates/cairo_air/src/lib.cairo line 251 at r1 (raw file):

    // The maximal constraint degree is 2, so the degree bound for the cairo air is the degree bound
    // of the trace plus 1.
    let cairo_air_log_degree_bound = tree_height - pcs_config.fri_config.log_blowup_factor + 1;

AlonF, Leo & Gali decided not to rename trace_log_size to tree_height

Gali-StarkWare

Reviewable status: 0 of 23 files reviewed, 4 unresolved discussions

stwo_cairo_verifier/crates/cairo_air/src/lib.cairo line 247 at r1 (raw file):

    let trace_log_size = *commitment_scheme.trees[1].tree_height;
    assert!(trace_log_size == *commitment_scheme.trees[2].tree_height);

Ignore, it's from the cherry-pick

Code quote:

    let trace_log_size = *commitment_scheme.trees[1].tree_height;
    assert!(trace_log_size == *commitment_scheme.trees[2].tree_height);

Gali-StarkWare

Reviewable status: 0 of 23 files reviewed, 5 unresolved discussions

stwo_cairo_verifier/crates/verifier_core/src/channel.cairo line 23 at r1 (raw file):

pub use feature_dependent_uses::*;

// TODO(audit): Remove ChannelTime and n_challenges. Done

Done in #1254

ilyalesokhin-starkware

Reviewable status: 0 of 23 files reviewed, 5 unresolved discussions

stwo_cairo_verifier/crates/verifier_core/src/vcs/blake2s_hasher.cairo line 26 at r1 (raw file):

        if column_values.is_empty() {
            // TODO(audit): use expect.
            let msg = match children_hashes {

I'm on it.

Code quote:

            // TODO(audit): use expect.
            let msg = match children_hashes {

Gali-StarkWare

Reviewable status: 0 of 23 files reviewed, 9 unresolved discussions (waiting on @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/fri.cairo line 137 at r1 (raw file):

        FriVerifier {
            config, first_layer, inner_layers, last_layer_domain: layer_domain, last_layer_poly,
            // TODO(audit): Remove queries from the struct. Done

Done in #1309

stwo_cairo_verifier/crates/verifier_core/src/channel/blake2s.cairo line 88 at r1 (raw file):

    }

    // TODO(audit): Delete this function. Reimplement mix_u64. Done

Done in #1316

stwo_cairo_verifier/crates/verifier_core/src/channel/blake2s.cairo line 186 at r1 (raw file):

///
/// Panics if `n_bits` >= 64.
// TODO(audit): Don't use the digest directly. Done

Done in #1327

stwo_cairo_verifier/crates/verifier_core/src/channel/poseidon252.cairo line 214 at r1 (raw file):

///
/// Panics if `n_bits` >= 64.
// TODO(audit): Don't use the digest directly, use a different hash for pow. Done

Done in #1327

Gali-StarkWare

Reviewable status: 0 of 23 files reviewed, 10 unresolved discussions (waiting on @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/channel/blake2s.cairo line 230 at r1 (raw file):

    // Append a zero byte for domain separation between generating randomness and mixing a
    // single u32.
    // TODO(audit): Add domain separation. E.g. change 36 to 37. Done

Done in #1328

leo-starkware

Reviewable status: 0 of 23 files reviewed, 11 unresolved discussions (waiting on @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/queries.cairo line 28 at r1 (raw file):

        while n_dict_entries != n_queries {
            // In each iteration, random_bytes is truncated to multiples of 4 bytes.
            // TODO(audit): Replace draw random bytes with draw u32s.

On it

Gali-StarkWare

Reviewable status: 0 of 23 files reviewed, 15 unresolved discussions (waiting on @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/pcs/quotients.cairo line 85 at r1 (raw file):

    }

    // TODO(audit): Assert that queried values have been consumed. Done

Done in #1311

stwo_cairo_verifier/crates/verifier_core/src/pcs/quotients.cairo line 274 at r1 (raw file):

            let neg_dbl_im_py = neg_twice_imaginary_part(sample_batch.point.y);
            // TODO(audit): Instead of starting by alpha = 1, start with the previous alpha from
            // last iteration and remove batch_random_coeff. Done

Done in #1349

stwo_cairo_verifier/crates/verifier_core/src/vcs/verifier.cairo line 193 at r1 (raw file):

        assert!(prev_layer_hashes.is_empty());

        // TODO(audit): Use assert. Done

Done in #1320

stwo_cairo_verifier/crates/verifier_core/src/vcs/poseidon_hasher.cairo line 24 at r1 (raw file):

            // Most often a node has no column values.
            if column_values.len() == 0 {
                // TODO(audit): Posiedon2 here { == hades_permutation(x, y, 2); } Done

Done in #1324

Gali-StarkWare

@Gali-StarkWare reviewed 1 of 23 files at r1.
Reviewable status: 1 of 23 files reviewed, 15 unresolved discussions (waiting on @ilyalesokhin-starkware)

Gali-StarkWare

Reviewable status: 1 of 23 files reviewed, 16 unresolved discussions (waiting on @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_utils/src/blake2s.cairo line 32 at r1 (raw file):

    let [d0, d1, d2, d3, d4, d5, d6, d7] = digest;

    // TODO(audit):use unwrap.

On this file

Gali-StarkWare

Reviewable status: 1 of 23 files reviewed, 16 unresolved discussions (waiting on @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_utils/src/blake2s.cairo line 32 at r1 (raw file):

Previously, Gali-StarkWare wrote…

On this file

All this file's fixes are in #1363

Gali-StarkWare

@Gali-StarkWare reviewed 1 of 23 files at r1.
Reviewable status: 2 of 23 files reviewed, 16 unresolved discussions (waiting on @ilyalesokhin-starkware)

ilyalesokhin-starkware · 2025-09-11T13:07:53Z

stwo_cairo_verifier/crates/verifier_core/src/fri.cairo line 102 at r1 (raw file):

        // TODO(audit): Change to for loop.
        // TODO(audit): Rename proof to layer_proof.

on it

Code quote:

        // TODO(audit): Change to column_commitment_domain.first().fold_to_line().
        let mut layer_domain = LineDomainImpl::new_unchecked(
            CosetImpl::half_odds(layer_log_bound + config.log_blowup_factor),
        );

        // TODO(audit): Change to for loop.
        // TODO(audit): Rename proof to layer_proof.

ilyalesokhin-starkware · 2025-09-11T13:12:17Z

stwo_cairo_verifier/crates/verifier_core/src/fri.cairo line 96 at r1 (raw file):

        let mut inner_layers = array![];
        let mut layer_log_bound = max_column_log_bound - CIRCLE_TO_LINE_FOLD_STEP;
        // TODO(audit): Change to column_commitment_domain.first().fold_to_line().

no such function.

Code quote:

 // TODO(audit): Change to column_commitment_domain.first().fold_to_line().

Gali-StarkWare

Reviewable status: 2 of 23 files reviewed, 20 unresolved discussions (waiting on @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/poly/utils.cairo line 37 at r1 (raw file):

//     let rhs_val = fold(values, folding_factors, index + n / 2, level + 1, n / 2);
//     return lhs_val + rhs_val.mul_m31(*folding_factors[level]);
// }

Moved to line_test.cairo in #1359

Code quote:

// pub fn fold(
//     values: @Array<SecureField>,
//     folding_factors: @Array<BaseField>,
//     index: usize,
//     level: usize,
//     n: usize,
// ) -> SecureField {
//     if n == 1 {
//         return *values[index];
//     }

//     let lhs_val = fold(values, folding_factors, index, level + 1, n / 2);
//     let rhs_val = fold(values, folding_factors, index + n / 2, level + 1, n / 2);
//     return lhs_val + rhs_val.mul_m31(*folding_factors[level]);
// }

stwo_cairo_verifier/crates/verifier_core/src/poly/line.cairo line 45 at r1 (raw file):

    //     fold(self.coeffs, @doublings, 0, 0, self.coeffs.len())
    // }

Moved to line_test.cairo in #1359

Code quote:

    // fn eval_at_point(self: @LinePoly, mut x: BaseField) -> SecureField {
    //     let mut doublings = array![];
    //     for _ in 0..*self.log_size {
    //         doublings.append(x);
    //         let x_square = x * x;
    //         x = x_square + x_square - m31(1);
    //     }

    //     fold(self.coeffs, @doublings, 0, 0, self.coeffs.len())
    // }

ilyalesokhin-starkware · 2025-09-11T13:30:20Z

stwo_cairo_verifier/crates/verifier_core/src/fri.cairo line 219 at r1 (raw file):

    // TODO(audit): Swap the order of the folds. In each iteration do the circle fold and line fold
    // to the same column size.

done

Code quote:

    // TODO(audit): Swap the order of the folds. In each iteration do the circle fold and line fold
    // to the same column size.

ilyalesokhin-starkware · 2025-09-11T13:30:29Z

stwo_cairo_verifier/crates/verifier_core/src/fri.cairo line 219 at r1 (raw file):

Previously, ilyalesokhin-starkware wrote…

done

sorry, on it

leo-starkware

Reviewable status: 2 of 23 files reviewed, 23 unresolved discussions (waiting on @Gali-StarkWare and @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/queries.cairo line 26 at r1 (raw file):

        let mut n_dict_entries = 0;
        let domain_size: NonZero<u32> = pow2(log_domain_size).try_into().unwrap();
        while n_dict_entries != n_queries {

On it (will include in draw_u32s PR)

stwo_cairo_verifier/crates/verifier_core/src/queries.cairo line 46 at r1 (raw file):

        // A squashed dict's entries are sorted by key in ascending order.
        let dict_entries = positions_dict.squash().into_entries();
        let mut sorted_positions: Array<u32> = array![];

On it (will include in draw_u32s PR)

ilyalesokhin-starkware · 2025-09-11T14:34:20Z

stwo_cairo_verifier/crates/verifier_core/src/fri.cairo line 219 at r1 (raw file):

Previously, ilyalesokhin-starkware wrote…

sorry, on it

https://reviewable.io/reviews/starkware-libs/stwo-cairo/1365

leo-starkware

Reviewable status: 2 of 23 files reviewed, 23 unresolved discussions (waiting on @Gali-StarkWare and @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/queries.cairo line 26 at r1 (raw file):

Previously, leo-starkware wrote…

On it (will include in draw_u32s PR)

#1366

stwo_cairo_verifier/crates/verifier_core/src/queries.cairo line 28 at r1 (raw file):

Previously, leo-starkware wrote…

On it

#1366

stwo_cairo_verifier/crates/verifier_core/src/queries.cairo line 46 at r1 (raw file):

Previously, leo-starkware wrote…

On it (will include in draw_u32s PR)

#1366

leo-starkware

Reviewable status: 2 of 23 files reviewed, 25 unresolved discussions (waiting on @Gali-StarkWare and @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/fri.cairo line 600 at r1 (raw file):

        let mut res = array![];

        // TODO(audit): Use zip_eq.

on it

stwo_cairo_verifier/crates/verifier_core/src/fri.cairo line 626 at r1 (raw file):

        let mut res = array![];

        // TODO(audit): Use zip_eq.

on it

Gali-StarkWare

Reviewable status: 2 of 23 files reviewed, 34 unresolved discussions (waiting on @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/fri.cairo line 117 at r1 (raw file):

                );

            layer_log_bound -= FOLD_STEP;

On it

stwo_cairo_verifier/crates/verifier_core/src/fri.cairo line 128 at r1 (raw file):

        assert!(
            last_layer_poly.log_size == config.log_last_layer_degree_bound,

Done in #1359

stwo_cairo_verifier/crates/verifier_core/src/fri.cairo line 150 at r1 (raw file):

    }

    // TODO(audit): Inline.

On it

stwo_cairo_verifier/crates/verifier_core/src/fri.cairo line 223 at r1 (raw file):

        let circle_poly_degree_bound = *layer.log_degree_bound + CIRCLE_TO_LINE_FOLD_STEP;

        // TODO(audit): Replace while with if (there's only one).

On it

stwo_cairo_verifier/crates/verifier_core/src/fri.cairo line 276 at r1 (raw file):

            "{}",
            FriVerificationError::LastLayerEvaluationsInvalid,
        );

Done in #1359

Code quote:

    assert!(last_layer_poly.coeffs.len() == 1);
    // let last_layer_evals = last_layer_poly.evaluate(last_layer_domain).values;
    // let domain_log_size = last_layer_domain.log_size();

    while let (Some(query), Some(query_eval)) =
        (queries.positions.pop_front(), query_evals.pop_front()) {
        // TODO(andrew): Makes more sense for the proof to provide coeffs in natural order and
        // the FFT return evals in bit-reversed order to prevent this unnecessary bit-reverse.
        // let last_layer_eval_i = bit_reverse_index(*query, domain_log_size);

        assert!(
            query_eval == *last_layer_poly.coeffs[0],
            "{}",
            FriVerificationError::LastLayerEvaluationsInvalid,
        );

stwo_cairo_verifier/crates/verifier_core/src/fri.cairo line 354 at r1 (raw file):

        let mut decommitted_values = array![];

        // TODO(audit): Remove.

On it

stwo_cairo_verifier/crates/verifier_core/src/fri.cairo line 363 at r1 (raw file):

        }

        // TODO(audit): Change to zip_eq.

On it

stwo_cairo_verifier/crates/verifier_core/src/fri.cairo line 529 at r1 (raw file):

    let mut folded_query_positions = queries.fold(fold_step).positions;

    // TODO(audit): Change to for loop.

On it

stwo_cairo_verifier/crates/verifier_core/src/fri.cairo line 91 at r1 (raw file):

        // Bounds are stored in descending order.
        // TODO(andrew): There is no check that it's sorted. Add check.

On it

Gali-StarkWare

Reviewable status: 2 of 23 files reviewed, 35 unresolved discussions (waiting on @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/fri.cairo line 536 at r1 (raw file):

        let mut subset_eval = array![];

        // TODO(audit): Change to for loop.

On it

leo-starkware

Reviewable status: 2 of 23 files reviewed, 50 unresolved discussions (waiting on @Gali-StarkWare and @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/vcs/verifier.cairo line 136 at r1 (raw file):

Previously, leo-starkware wrote…

on it

@ilyalesokhin-starkware Does None happen only in the last log_blowup_factor iterations? i.e. we have columns for all the sizes under all cirumstances?

stwo_cairo_verifier/crates/verifier_core/src/vcs/verifier.cairo line 156 at r1 (raw file):

Previously, leo-starkware wrote…

on it

#1372

stwo_cairo_verifier/crates/verifier_core/src/vcs/verifier.cairo line 227 at r1 (raw file):

) -> Option<@H> {
    // If the child was computed, use that value.
    if let Some((q, h)) = prev_layer_hashes.span().first()

#1372

leo-starkware

Reviewable status: 2 of 23 files reviewed, 52 unresolved discussions (waiting on @Gali-StarkWare and @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/vcs/verifier.cairo line 119 at r1 (raw file):

Previously, leo-starkware wrote…

on it

#1373

stwo_cairo_verifier/crates/verifier_core/src/vcs/verifier.cairo line 124 at r1 (raw file):

Previously, leo-starkware wrote…

on it

#1373

stwo_cairo_verifier/crates/verifier_core/src/vcs/verifier.cairo line 175 at r1 (raw file):

                    queried_values.pop_front_n(n_columns_in_layer)
                } else {
                    // TODO(audit): Remove this and add assert that n_columns_in_layer == 0.

#1373

stwo_cairo_verifier/crates/verifier_core/src/vcs/verifier.cairo line 187 at r1 (raw file):

        // Check that all witnesses and values have been consumed.
        // TODO(audit): Check also queried_values and prev_layer_hashes.

#1373

leo-starkware

Reviewable status: 2 of 23 files reviewed, 53 unresolved discussions (waiting on @Gali-StarkWare and @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/verifier.cairo line 97 at r1 (raw file):

    mask: TreeSpan<ColumnSpan<Span<QM31>>>,
) -> Result<QM31, InvalidOodsSampleStructure> {
    // TODO(audit): Use panic instead of result (unwrap).

on it

Gali-StarkWare

@Gali-StarkWare reviewed 3 of 23 files at r1.
Reviewable status: 5 of 23 files reviewed, 53 unresolved discussions (waiting on @ilyalesokhin-starkware)

Gali-StarkWare

Reviewable status: 5 of 23 files reviewed, 54 unresolved discussions (waiting on @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/verifier.cairo line 48 at r1 (raw file):

    // Check that there are enough security bits.
    // TODO(audit): Add another lower bound on pow bits.

On it

leo-starkware

Reviewable status: 5 of 23 files reviewed, 54 unresolved discussions (waiting on @Gali-StarkWare and @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/channel/blake2s.cairo line 160 at r1 (raw file):

Previously, leo-starkware wrote…

On it

#1374

alon-f

Reviewable status: 5 of 23 files reviewed, 54 unresolved discussions (waiting on @Gali-StarkWare and @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/utils.cairo line 216 at r1 (raw file):

Previously, alon-f wrote…

On it

#1371

stwo_cairo_verifier/crates/verifier_core/src/utils.cairo line 306 at r1 (raw file):

Previously, alon-f wrote…

On it

#1375

Gali-StarkWare

Reviewable status: 5 of 23 files reviewed, 54 unresolved discussions (waiting on @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/verifier.cairo line 48 at r1 (raw file):

Previously, Gali-StarkWare wrote…

On it

#1376

alon-f

Reviewable status: 5 of 23 files reviewed, 54 unresolved discussions (waiting on @Gali-StarkWare and @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/verifier.cairo line 56 at r1 (raw file):

Previously, alon-f wrote…

On it

#1377

alon-f

Reviewable status: 5 of 23 files reviewed, 55 unresolved discussions (waiting on @Gali-StarkWare and @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/fields/cm31/naive.cairo line 118 at r1 (raw file):

    #[inline]
    fn into(self: M31) -> CM31 {
        CM31 { a: self, b: Zero::zero() }

On it

ilyalesokhin-starkware · 2025-09-15T08:32:34Z

stwo_cairo_verifier/crates/cairo_air/src/lib.cairo line 251 at r1 (raw file):

Previously, Gali-StarkWare wrote…

AlonF, Leo & Gali decided not to rename trace_log_size to tree_height

this code need to be refactored after we bump the compiler version.

ilyalesokhin-starkware · 2025-09-15T08:33:28Z

stwo_cairo_verifier/crates/verifier_core/src/fri.cairo line 219 at r1 (raw file):

Previously, ilyalesokhin-starkware wrote…

https://reviewable.io/reviews/starkware-libs/stwo-cairo/1365

done

ilyalesokhin-starkware · 2025-09-15T08:38:30Z

stwo_cairo_verifier/crates/verifier_core/src/pcs/verifier.cairo line 152 at r1 (raw file):

            // The Merkle implementation pops values from the query position dict so it has to
            // be duplicated.
            // TODO(audit): Remove clone_subset, pass dict by reference.

on it

Code quote:

// TODO(audit): Remove clone_subset, pass dict by reference.

alon-f

Reviewable status: 5 of 23 files reviewed, 55 unresolved discussions (waiting on @Gali-StarkWare and @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/fields/cm31/naive.cairo line 118 at r1 (raw file):

Previously, alon-f wrote…

On it

#1378

alon-f

Reviewable status: 5 of 23 files reviewed, 56 unresolved discussions (waiting on @Gali-StarkWare and @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/fields/cm31/opcode.cairo line 12 at r1 (raw file):

pub struct CM31 {
    // Represented using QM31, since QM31 has a dedicated opcode.
    inner: super::super::qm31::QM31,

On it

leo-starkware

Reviewable status: 5 of 23 files reviewed, 56 unresolved discussions (waiting on @Gali-StarkWare and @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/verifier.cairo line 97 at r1 (raw file):

Previously, leo-starkware wrote…

on it

#1379

alon-f

Reviewable status: 5 of 23 files reviewed, 56 unresolved discussions (waiting on @Gali-StarkWare and @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/fields/cm31/opcode.cairo line 12 at r1 (raw file):

Previously, alon-f wrote…

On it

#1380

leo-starkware

Reviewable status: 5 of 23 files reviewed, 58 unresolved discussions (waiting on @Gali-StarkWare and @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/channel/poseidon252.cairo line 66 at r1 (raw file):

                },
                Option::None => {
                    // TODO(audit): Use if let.

Done by @Gali-StarkWare in #1357

stwo_cairo_verifier/crates/verifier_core/src/channel/poseidon252.cairo line 218 at r1 (raw file):

    let u256 { low, .. } = digest.into();
    let two_pow_n_bits: u128 = pow2_u64(n_bits).into();
    let nonzero_divisor: NonZero<u128> = two_pow_n_bits.try_into().unwrap();

Done by @Gali-StarkWare in #1357

alon-f

Reviewable status: 5 of 23 files reviewed, 59 unresolved discussions (waiting on @Gali-StarkWare and @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/pcs/quotients.cairo line 57 at r1 (raw file):

        // Collect the column samples and the number of columns in each tree.
        let mut samples: Array<@Array<PointSample>> = array![];

On it

ilyalesokhin-starkware · 2025-09-15T12:33:22Z

stwo_cairo_verifier/crates/verifier_core/src/pcs/verifier.cairo line 152 at r1 (raw file):

Previously, ilyalesokhin-starkware wrote…

on it

#1381

ilyalesokhin-starkware · 2025-09-15T12:42:35Z

stwo_cairo_verifier/crates/verifier_core/src/vcs/blake2s_hasher.cairo line 21 at r1 (raw file):

    ) -> Self::Hash {
        let mut state = BoxImpl::new(BLAKE2S_256_INITIAL_STATE);
        // TODO(audit): Consider domain separation.

done in
#1361

Code quote:

// TODO(audit): Consider domain separation.

ilyalesokhin-starkware · 2025-09-15T12:42:51Z

stwo_cairo_verifier/crates/verifier_core/src/vcs/blake2s_hasher.cairo line 26 at r1 (raw file):

Previously, ilyalesokhin-starkware wrote…

I'm on it.

rafactored in #1361

leo-starkware

Reviewable status: 5 of 23 files reviewed, 66 unresolved discussions (waiting on @Gali-StarkWare and @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/vcs/blake2s_hasher.cairo line 96 at r1 (raw file):

            padded_values.append(0);
        }
        let msg: Box<[u32; 16]> = *padded_values.span().try_into().unwrap();

on it

stwo_cairo_verifier/crates/verifier_core/src/channel/blake2s.cairo line 192 at r1 (raw file):

    let v = d1.into() * U64_2_POW_32 + d0.into();

    let nonzero_divisor: NonZero<u64> = pow2_u64(n_bits).try_into().unwrap();

on it

stwo_cairo_verifier/crates/verifier_core/src/pcs/verifier.cairo line 110 at r1 (raw file):

            sampled_values,
            decommitments,
            queried_values: queried_values_per_tree,

on it

stwo_cairo_verifier/crates/verifier_core/src/pcs/verifier.cairo line 148 at r1 (raw file):

        // Verify Merkle decommitments.
        for (tree, (queried_values, decommitment)) in zip_eq(
            self.trees.span(), zip_eq(queried_values_per_tree.span(), decommitments),

on it

stwo_cairo_verifier/crates/verifier_core/src/pcs/verifier.cairo line 167 at r1 (raw file):

            random_coeff,
            query_positions_by_log_size,
            queried_values_per_tree.span(),

on it

stwo_cairo_verifier/crates/verifier_core/src/vcs/poseidon_hasher.cairo line 36 at r1 (raw file):

                // Inlined and simplified `poseidon_hash_span(...)` for better performance.
                let [v0, v1, v2, v3]: [BaseField; QM31_EXTENSION_DEGREE] = (*values).unbox();
                let mut word: felt252 = v0.inner.into();

on it

leo-starkware

Reviewable status: 5 of 23 files reviewed, 66 unresolved discussions (waiting on @Gali-StarkWare and @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/channel/blake2s.cairo line 192 at r1 (raw file):

Previously, leo-starkware wrote…

on it

#1387

stwo_cairo_verifier/crates/verifier_core/src/pcs/verifier.cairo line 110 at r1 (raw file):

Previously, leo-starkware wrote…

on it

#1387

stwo_cairo_verifier/crates/verifier_core/src/pcs/verifier.cairo line 148 at r1 (raw file):

Previously, leo-starkware wrote…

on it

#1387

stwo_cairo_verifier/crates/verifier_core/src/pcs/verifier.cairo line 167 at r1 (raw file):

Previously, leo-starkware wrote…

on it

#1387

stwo_cairo_verifier/crates/verifier_core/src/vcs/poseidon_hasher.cairo line 36 at r1 (raw file):

Previously, leo-starkware wrote…

on it

#1387

ilyalesokhin-starkware · 2025-09-21T08:56:28Z

stwo_cairo_verifier/crates/cairo_air/src/lib.cairo line 251 at r1 (raw file):

Previously, ilyalesokhin-starkware wrote…

this code need to be refactored after we bump the compiler version.

done in #1389

ilyalesokhin-starkware · 2025-09-21T08:58:04Z

stwo_cairo_verifier/crates/verifier_core/src/pcs/verifier.cairo line 38 at r1 (raw file):

pub struct CommitmentSchemeVerifier {
    pub trees: TreeArray<MerkleVerifier<MerkleHasher>>,
    // TODO(audit): Remove the config.

done in #1396

Code quote:

 // TODO(audit): Remove the config.

Gali-StarkWare

Reviewable status: 5 of 23 files reviewed, 66 unresolved discussions (waiting on @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/fri.cairo line 102 at r1 (raw file):

Previously, ilyalesokhin-starkware wrote…

on it

Done by Ilya in #1364

Gali-StarkWare

Reviewable status: 5 of 23 files reviewed, 65 unresolved discussions (waiting on @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/verifier.cairo line 48 at r1 (raw file):

Previously, Gali-StarkWare wrote…

#1376

Done in #1388 instead

Gali-StarkWare

Reviewable status: 5 of 23 files reviewed, 67 unresolved discussions (waiting on @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/vcs/blake2s_hasher.cairo line 36 at r1 (raw file):

            // This is acceptable because the verifier always knows
            // the exact structure of the Merkle tree.
            return Blake2sHash { hash: blake2s_finalize(:state, byte_count: 64, :msg) };

I see that in #1361 you finalize with byte_count = 128, is that on purpose (for domain separation)?

Code quote:

  return Blake2sHash { hash: blake2s_finalize(:state, byte_count: 64, :msg) };

Gali-StarkWare

Reviewable status: 5 of 23 files reviewed, 67 unresolved discussions (waiting on @ilyalesokhin-starkware)

stwo_cairo_verifier/crates/verifier_core/src/vcs/blake2s_hasher.cairo line 36 at r1 (raw file):

Previously, Gali-StarkWare wrote…

I see that in #1361 you finalize with byte_count = 128, is that on purpose (for domain separation)?

@ilyalesokhin-starkware

anatgstarkware and others added 2 commits September 11, 2025 11:53

Verifier core audit comments

078fe75

Audit comments

5ef3dd3

Gali-StarkWare commented Sep 11, 2025

View reviewed changes

ilyalesokhin-starkware reviewed Sep 11, 2025

View reviewed changes

Gali-StarkWare requested a review from ilyalesokhin-starkware September 11, 2025 10:36

Gali-StarkWare commented Sep 11, 2025

View reviewed changes

leo-starkware reviewed Sep 11, 2025

View reviewed changes

Gali-StarkWare commented Sep 11, 2025

View reviewed changes

leo-starkware reviewed Sep 11, 2025

View reviewed changes

leo-starkware reviewed Sep 14, 2025

View reviewed changes

Gali-StarkWare commented Sep 14, 2025

View reviewed changes

leo-starkware reviewed Sep 14, 2025

View reviewed changes

leo-starkware reviewed Sep 15, 2025

View reviewed changes

Gali-StarkWare commented Sep 15, 2025

View reviewed changes

leo-starkware reviewed Sep 15, 2025

View reviewed changes

alon-f reviewed Sep 15, 2025

View reviewed changes

Gali-StarkWare commented Sep 15, 2025

View reviewed changes

alon-f reviewed Sep 15, 2025

View reviewed changes

leo-starkware reviewed Sep 15, 2025

View reviewed changes

alon-f reviewed Sep 15, 2025

View reviewed changes

leo-starkware reviewed Sep 15, 2025

View reviewed changes

alon-f reviewed Sep 15, 2025

View reviewed changes

leo-starkware reviewed Sep 16, 2025

View reviewed changes

Gali-StarkWare commented Sep 21, 2025

View reviewed changes

Verifier core audit comments #1360

Are you sure you want to change the base?

Verifier core audit comments #1360

Uh oh!

Conversation

Gali-StarkWare commented Sep 11, 2025 • edited by RoeiE-starkware Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Gali-StarkWare commented Sep 11, 2025

Uh oh!

Gali-StarkWare left a comment

Choose a reason for hiding this comment

Uh oh!

Gali-StarkWare left a comment

Choose a reason for hiding this comment

Uh oh!

Gali-StarkWare left a comment

Choose a reason for hiding this comment

Uh oh!

Gali-StarkWare left a comment

Choose a reason for hiding this comment

Uh oh!

Gali-StarkWare left a comment

Choose a reason for hiding this comment

Uh oh!

Gali-StarkWare left a comment

Choose a reason for hiding this comment

Uh oh!

ilyalesokhin-starkware left a comment

Choose a reason for hiding this comment

Uh oh!

Gali-StarkWare left a comment

Choose a reason for hiding this comment

Uh oh!

Gali-StarkWare left a comment

Choose a reason for hiding this comment

Uh oh!

leo-starkware left a comment

Choose a reason for hiding this comment

Uh oh!

Gali-StarkWare left a comment

Choose a reason for hiding this comment

Uh oh!

Gali-StarkWare left a comment

Choose a reason for hiding this comment

Uh oh!

Gali-StarkWare left a comment

Choose a reason for hiding this comment

Uh oh!

Gali-StarkWare left a comment

Choose a reason for hiding this comment

Uh oh!

Gali-StarkWare left a comment

Choose a reason for hiding this comment

Uh oh!

ilyalesokhin-starkware commented Sep 11, 2025

Uh oh!

ilyalesokhin-starkware commented Sep 11, 2025

Uh oh!

Gali-StarkWare left a comment

Choose a reason for hiding this comment

Uh oh!

ilyalesokhin-starkware commented Sep 11, 2025

Uh oh!

ilyalesokhin-starkware commented Sep 11, 2025

Uh oh!

leo-starkware left a comment

Choose a reason for hiding this comment

Uh oh!

ilyalesokhin-starkware commented Sep 11, 2025

Uh oh!

leo-starkware left a comment

Choose a reason for hiding this comment

Uh oh!

leo-starkware left a comment

Choose a reason for hiding this comment

Uh oh!

Gali-StarkWare left a comment

Choose a reason for hiding this comment

Uh oh!

Gali-StarkWare commented Sep 11, 2025 •

edited by RoeiE-starkware

Loading