Skip to content

Commit 9c9638b

Browse files
authored
fix: Validate length of proof.commit_phase_commits (#1468)
The length of `proof.commit_phase_commits` is not validated and [here](https://github.com/openvm-org/openvm/blob/18096f4194b76b743463c0fb39955f24d010e9bf/extensions/native/recursion/src/fri/two_adic_pcs.rs#L125) we directly use it as the maximum log trace height. close INT-3646
1 parent a1c1087 commit 9c9638b

File tree

4 files changed

+52
-21
lines changed

4 files changed

+52
-21
lines changed

extensions/native/recursion/src/commit.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ pub trait PcsVariable<C: Config> {
5454
builder: &mut Builder<C>,
5555
rounds: Array<C, TwoAdicPcsRoundVariable<C>>,
5656
proof: Self::Proof,
57+
log_max_height: RVar<C::N>,
5758
challenger: &mut impl ChallengerVariable<C>,
5859
);
5960
}

extensions/native/recursion/src/fri/mod.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,22 +39,23 @@ pub fn verify_query<C: Config>(
3939
proof: &FriQueryProofVariable<C>,
4040
betas: &Array<C, Ext<C::F, C::EF>>,
4141
reduced_openings: &Array<C, Ext<C::F, C::EF>>,
42-
log_max_height: RVar<C::N>,
42+
log_max_lde_height: RVar<C::N>,
4343
) -> Ext<C::F, C::EF>
4444
where
4545
C::F: TwoAdicField,
4646
C::EF: TwoAdicField,
4747
{
4848
builder.cycle_tracker_start("verify-query");
4949
let folded_eval: Ext<C::F, C::EF> = builder.eval(C::F::ZERO);
50-
let two_adic_generator_f = config.get_two_adic_generator(builder, log_max_height);
50+
let two_adic_generator_f = config.get_two_adic_generator(builder, log_max_lde_height);
5151

5252
let two_adic_gen_ext = two_adic_generator_f.to_operand().symbolic();
5353
let two_adic_generator_ef: Ext<_, _> = builder.eval(two_adic_gen_ext);
5454

55-
let index_bits_truncated = index_bits.slice(builder, 0, log_max_height);
55+
let index_bits_truncated = index_bits.slice(builder, 0, log_max_lde_height);
5656
let x = builder.exp_bits_big_endian(two_adic_generator_ef, &index_bits_truncated);
5757

58+
// proof.commit_phase_openings.len() == log_max_lde_height - log_blowup
5859
builder.assert_usize_eq(
5960
proof.commit_phase_openings.len(),
6061
commit_phase_commits.len(),
@@ -63,12 +64,13 @@ where
6364
.range(0, commit_phase_commits.len())
6465
.for_each(|i_vec, builder| {
6566
let i = i_vec[0];
66-
let log_folded_height = builder.eval_expr(log_max_height - i - C::N::ONE);
67+
let log_folded_height = builder.eval_expr(log_max_lde_height - i - C::N::ONE);
6768
let log_folded_height_plus_one = builder.eval_expr(log_folded_height + C::N::ONE);
6869
let commit = builder.get(commit_phase_commits, i);
6970
let step = builder.get(&proof.commit_phase_openings, i);
7071
let beta = builder.get(betas, i);
7172

73+
// reduced_openings.len() == MAX_TWO_ADICITY >= log_max_lde_height >= log_folded_height_plus_one
7274
let reduced_opening = builder.get(reduced_openings, log_folded_height_plus_one);
7375
builder.assign(&folded_eval, folded_eval + reduced_opening);
7476

extensions/native/recursion/src/fri/two_adic_pcs.rs

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ pub const MAX_TWO_ADICITY: usize = 27;
2525
/// 1. FieldMerkleTreeMMCS sorts traces by height in descending order when committing data.
2626
/// 2. **Required** that `C::F` has two-adicity <= [MAX_TWO_ADICITY]. In particular this implies that all LDE matrices have
2727
/// `log2(lde_height) <= MAX_TWO_ADICITY`.
28+
/// 3. **Required** that the maximum trace height is `2^log_max_height - 1`.
2829
///
2930
/// Reference:
3031
/// <https://github.com/Plonky3/Plonky3/blob/27b3127dab047e07145c38143379edec2960b3e1/merkle-tree/src/merkle_tree.rs#L53>
@@ -41,6 +42,7 @@ pub fn verify_two_adic_pcs<C: Config>(
4142
config: &FriConfigVariable<C>,
4243
rounds: Array<C, TwoAdicPcsRoundVariable<C>>,
4344
proof: FriProofVariable<C>,
45+
log_max_height: RVar<C::N>,
4446
challenger: &mut impl ChallengerVariable<C>,
4547
) where
4648
C::F: TwoAdicField,
@@ -101,7 +103,10 @@ pub fn verify_two_adic_pcs<C: Config>(
101103
}
102104

103105
builder.cycle_tracker_start("stage-d-verifier-verify");
104-
let betas: Array<C, Ext<C::F, C::EF>> = builder.array(proof.commit_phase_commits.len());
106+
// **ATTENTION**: always check shape of user inputs.
107+
builder.assert_usize_eq(proof.query_proofs.len(), RVar::from(config.num_queries));
108+
builder.assert_usize_eq(proof.commit_phase_commits.len(), log_max_height);
109+
let betas: Array<C, Ext<C::F, C::EF>> = builder.array(log_max_height);
105110
iter_zip!(builder, proof.commit_phase_commits, betas).for_each(|ptr_vec, builder| {
106111
let comm_ptr = ptr_vec[0];
107112
let beta_ptr = ptr_vec[1];
@@ -117,16 +122,12 @@ pub fn verify_two_adic_pcs<C: Config>(
117122
challenger.observe_slice(builder, final_poly_elem_felts);
118123
});
119124

120-
// **ATTENTION**: always check shape of user inputs.
121-
builder.assert_usize_eq(proof.query_proofs.len(), RVar::from(config.num_queries));
122-
123125
challenger.check_witness(builder, config.proof_of_work_bits, proof.pow_witness);
124126

125-
let log_max_height =
126-
builder.eval_expr(proof.commit_phase_commits.len() + RVar::from(log_blowup));
127+
let log_max_lde_height = builder.eval_expr(log_max_height + RVar::from(log_blowup));
127128
// tag_exp is a shared buffer.
128-
let tag_exp: Array<C, Felt<C::F>> = builder.array(log_max_height);
129-
let w = config.get_two_adic_generator(builder, log_max_height);
129+
let tag_exp: Array<C, Felt<C::F>> = builder.array(log_max_lde_height);
130+
let w = config.get_two_adic_generator(builder, log_max_lde_height);
130131
let max_gen_pow = config.get_two_adic_generator(builder, 1);
131132
let one_var: Felt<C::F> = builder.eval(C::F::ONE);
132133

@@ -154,7 +155,7 @@ pub fn verify_two_adic_pcs<C: Config>(
154155

155156
iter_zip!(builder, proof.query_proofs).for_each(|ptr_vec, builder| {
156157
let query_proof = builder.iter_ptr_get(&proof.query_proofs, ptr_vec[0]);
157-
let index_bits = challenger.sample_bits(builder, log_max_height);
158+
let index_bits = challenger.sample_bits(builder, log_max_lde_height);
158159

159160
// We reset the reduced opening accumulators at the start of each query.
160161
// We describe what `ro[log_height]` computes per query in pseduo-code, where `log_height` is log2 of the size of the LDE domain:
@@ -199,7 +200,7 @@ pub fn verify_two_adic_pcs<C: Config>(
199200
builder.cycle_tracker_start("cache-generator-powers");
200201
{
201202
// truncate index_bits to log_max_height
202-
let index_bits_truncated = index_bits.slice(builder, 0, log_max_height);
203+
let index_bits_truncated = index_bits.slice(builder, 0, log_max_lde_height);
203204

204205
// b = index_bits
205206
// w = generator of order 2^log_max_height
@@ -262,7 +263,7 @@ pub fn verify_two_adic_pcs<C: Config>(
262263
let cur_alpha_pow = builder.get(&alpha_pow, log_height);
263264

264265
builder.cycle_tracker_start("exp-reverse-bits-len");
265-
let height_idx = builder.eval_expr(log_max_height - log_height);
266+
let height_idx = builder.eval_expr(log_max_lde_height - log_height);
266267
let x = builder.get(&tag_exp, height_idx);
267268
builder.cycle_tracker_end("exp-reverse-bits-len");
268269

@@ -317,7 +318,8 @@ pub fn verify_two_adic_pcs<C: Config>(
317318
});
318319
builder.cycle_tracker_end("compute-reduced-opening");
319320

320-
let bits_reduced: Usize<_> = builder.eval(log_max_height - log_batch_max_height);
321+
let bits_reduced: Usize<_> =
322+
builder.eval(log_max_lde_height - log_batch_max_height);
321323
let index_bits_shifted_v1 = index_bits.shift(builder, bits_reduced);
322324

323325
builder.cycle_tracker_start("verify-batch");
@@ -341,7 +343,7 @@ pub fn verify_two_adic_pcs<C: Config>(
341343
&query_proof,
342344
&betas,
343345
&ro,
344-
log_max_height,
346+
log_max_lde_height,
345347
);
346348

347349
builder.assert_ext_eq(folded_eval, final_poly_ct);
@@ -435,9 +437,17 @@ where
435437
builder: &mut Builder<C>,
436438
rounds: Array<C, TwoAdicPcsRoundVariable<C>>,
437439
proof: Self::Proof,
440+
log_max_height: RVar<C::N>,
438441
challenger: &mut impl ChallengerVariable<C>,
439442
) {
440-
verify_two_adic_pcs(builder, &self.config, rounds, proof, challenger)
443+
verify_two_adic_pcs(
444+
builder,
445+
&self.config,
446+
rounds,
447+
proof,
448+
log_max_height,
449+
challenger,
450+
)
441451
}
442452
}
443453

@@ -691,7 +701,13 @@ pub mod tests {
691701
let commit = DigestVariable::Felt(builder.constant::<Array<_, _>>(commit));
692702
challenger.observe_digest(&mut builder, commit);
693703
challenger.sample_ext(&mut builder);
694-
pcs_var.verify(&mut builder, rounds, proofvar, &mut challenger);
704+
pcs_var.verify(
705+
&mut builder,
706+
rounds,
707+
proofvar,
708+
RVar::from(nb_log2_rows),
709+
&mut challenger,
710+
);
695711
builder.halt();
696712

697713
let program = builder.compile_isa();

extensions/native/recursion/src/stark/mod.rs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,12 @@ where
200200
};
201201
// (T02a): `air_perm_by_height` is a valid permutation of `0..num_airs`.
202202
// (T02b): For all `i`, `air_proofs[i].log_degree <= MAX_TWO_ADICITY - log_blowup`.
203+
// (T02c): For all `0<=i<num_air-1`, `air_proofs[air_perm_by_height[i]].log_degree >= air_proofs[air_perm_by_height[i+1]].log_degree`.
204+
let log_max_height = {
205+
let index = builder.get(air_perm_by_height, RVar::zero());
206+
let air_proof = builder.get(air_proofs, index);
207+
RVar::from(air_proof.log_degree.clone())
208+
};
203209

204210
// OK: trace_height_constraint_system comes from vkey so requirements of
205211
// `check_trace_height_constraints` are met.
@@ -375,7 +381,7 @@ where
375381
RVar::from(builder.sll::<Usize<_>>(RVar::one(), log_quotient_degree));
376382
let log_quotient_size = builder.eval_expr(log_degree + log_quotient_degree);
377383
// Assumption: (T02b) `log_degree <= MAX_TWO_ADICITY - low_blowup`
378-
// Because the VK ensures `log_quotient_degree < log_blowup`, this won't access an out
384+
// Because the VK ensures `log_quotient_degree <= log_blowup`, this won't access an out
379385
// of bound index.
380386
let quotient_domain =
381387
domain.create_disjoint_domain(builder, log_quotient_size, Some(pcs.config.clone()));
@@ -683,7 +689,13 @@ where
683689

684690
// Verify the pcs proof
685691
builder.cycle_tracker_start("stage-d-verify-pcs");
686-
pcs.verify(builder, rounds, opening.proof.clone(), challenger);
692+
pcs.verify(
693+
builder,
694+
rounds,
695+
opening.proof.clone(),
696+
log_max_height,
697+
challenger,
698+
);
687699
builder.cycle_tracker_end("stage-d-verify-pcs");
688700

689701
builder.cycle_tracker_start("stage-e-verify-constraints");

0 commit comments

Comments
 (0)